MCSaveVersionDialog splitter broken

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

MCSaveVersionDialog splitter broken

Nicolas Cellier
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...


Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

Christoph Thiede

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...


Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

Nicolas Cellier
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...



Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

Nicolas Cellier
Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...



Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

marcel.taeumel
Hi, there.

I cannot solve this issue at the moment.

Here is some history about (the rather recently added) #balanceOffsets:

http://forum.world.st/A-fix-for-ProportionalSplitterMorph-td5072182.html
http://forum.world.st/A-fix-for-ProportionalSplitterMorph-take-2-td5072239.html

one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds

That's exactly what's happening. See:

Morph >> #doLayoutIn:
ProportionalLayout >> #layoutIn:
Morph >> #layoutProportionallyInBounds:(positioning:)
LayoutFrame >> layout:in:

for example the grip morphs have a very strange idea of their own layoutFrame

Yes, grip morphs are quite hacky. Their relationship to SystemWindow and that window's label area required some unconventional overrides of #layoutProportionallyInBounds:(positioning:)

Anyway, that issue with #balanceOffsets: does not bleed into all these things. It is just confined within ProportionalSplitterMorph. I suppose. :-D

Best,
Marcel

Am 17.11.2019 23:34:24 schrieb Nicolas Cellier <[hidden email]>:

Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...



Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

Stéphane Rollandin
> Here is some history about (the rather recently added) #balanceOffsets:
>
> http://forum.world.st/A-fix-for-ProportionalSplitterMorph-td5072182.html
> http://forum.world.st/A-fix-for-ProportionalSplitterMorph-take-2-td5072239.html

Indeed, #balanceOffsets is an attempt to fix the problem described by
Nicolas in #repositionBy: that I proposed because it worked for me (at
the time I needed correct splitters for Boadebui, my MIDI sequencer:
http://www.zogotounga.net/comp/boadebui.htm).

But it messed with Smart Splitters, and so Chris Muller and I had some
private exchanges and the code eventually became what it is now.

But the root of the problem is in #repositionBy: and ideally we should
not need #balanceOffsets at all.

Stef

Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

Chris Muller-3
In reply to this post by Nicolas Cellier
Hi Nicolas,

I don't really understand the symptom you describe, but if you yellow-clicked the splitter, then that would've invoked the manual smart-splitter feature.

It takes care to follow certain principles which require absolute positioning relative to other widgets.  For example, the desire to reduce the number of scroll bars.  Another is to avoid stealing focus when mouseOverForKeyboard focus is used.  These requirements require a duality of proportional and absolute positioning control.  I made a video a while ago which presents the feature and covers this point. [1]

Best,
  Chris


On Sun, Nov 17, 2019 at 4:34 PM Nicolas Cellier <[hidden email]> wrote:
Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...




Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

Chris Muller-3
In reply to this post by marcel.taeumel
On Mon, Nov 18, 2019 at 2:20 AM Marcel Taeumel <[hidden email]> wrote:
Hi, there.

I cannot solve this issue at the moment.

Here is some history about (the rather recently added) #balanceOffsets:


one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds

That's exactly what's happening. See:

Morph >> #doLayoutIn:
ProportionalLayout >> #layoutIn:
Morph >> #layoutProportionallyInBounds:(positioning:)
LayoutFrame >> layout:in:

for example the grip morphs have a very strange idea of their own layoutFrame

Yes, grip morphs are quite hacky. Their relationship to SystemWindow and that window's label area required some unconventional overrides of #layoutProportionallyInBounds:(positioning:)

Anyway, that issue with #balanceOffsets: does not bleed into all these things. It is just confined within ProportionalSplitterMorph. I suppose. :-D

Yes, it appears I isolated balanceOffsets by the preference checks:

    (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ])

and in ProportionalSplitterMorph>>#stopStepping, it once again calls #balanceOffsets to put the state back to the manual-splitter scheme, as if the user had moved the bar themself.

 - Chris




Best,
Marcel

Am 17.11.2019 23:34:24 schrieb Nicolas Cellier <[hidden email]>:

Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...




Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

marcel.taeumel
This is the bug (smart splitters disabled, left mouse click):

Best,
Marcel

Am 20.11.2019 05:40:59 schrieb Chris Muller <[hidden email]>:

On Mon, Nov 18, 2019 at 2:20 AM Marcel Taeumel <[hidden email]> wrote:
Hi, there.

I cannot solve this issue at the moment.

Here is some history about (the rather recently added) #balanceOffsets:


one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds

That's exactly what's happening. See:

Morph >> #doLayoutIn:
ProportionalLayout >> #layoutIn:
Morph >> #layoutProportionallyInBounds:(positioning:)
LayoutFrame >> layout:in:

for example the grip morphs have a very strange idea of their own layoutFrame

Yes, grip morphs are quite hacky. Their relationship to SystemWindow and that window's label area required some unconventional overrides of #layoutProportionallyInBounds:(positioning:)

Anyway, that issue with #balanceOffsets: does not bleed into all these things. It is just confined within ProportionalSplitterMorph. I suppose. :-D

Yes, it appears I isolated balanceOffsets by the preference checks:

    (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ])

and in ProportionalSplitterMorph>>#stopStepping, it once again calls #balanceOffsets to put the state back to the manual-splitter scheme, as if the user had moved the bar themself.

 - Chris




Best,
Marcel

Am 17.11.2019 23:34:24 schrieb Nicolas Cellier <[hidden email]>:

Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...




Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

Nicolas Cellier
Thanks Marcel!
how long does it take to produce such animation?

Chris, what is the purpose of fractional layouts?
It is to provide adaptive behavior to window resizing.
LayoutFrame enable having a mixture of fixed size widgets, variable size widgets with size proportional to window extent, or mixed case.
Patching the layout with absolute offsets, is a big mistake, because the layout then misbehave when window is resized, especially when shrinked.
The offsets are not proportional, and they are not meant to be, they are fixed width constraints!
Patching the offset also override the fixed width specification, which is another mistake: fixed size constraint information is lost.

What smart splitters do? They are introducing a new rule: size is proportional to contents.
It may express some constraints in term of absolute positioning/extent, that's not a problem.
What I deny is the fact that it would resolve those constraints by changing absolute offsets.
Smart splitters shall adapt the proportional layout of widgets, that's crystal clear.

The problem I exposed is not related to smart splitters, just to the way regular splitter (miss)-work.
We should first solve that, having a reasonable behavior and simple implementation, then we will come back to smart splitters.


Le mer. 20 nov. 2019 à 09:02, Marcel Taeumel <[hidden email]> a écrit :
This is the bug (smart splitters disabled, left mouse click):

Best,
Marcel

Am 20.11.2019 05:40:59 schrieb Chris Muller <[hidden email]>:

On Mon, Nov 18, 2019 at 2:20 AM Marcel Taeumel <[hidden email]> wrote:
Hi, there.

I cannot solve this issue at the moment.

Here is some history about (the rather recently added) #balanceOffsets:


one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds

That's exactly what's happening. See:

Morph >> #doLayoutIn:
ProportionalLayout >> #layoutIn:
Morph >> #layoutProportionallyInBounds:(positioning:)
LayoutFrame >> layout:in:

for example the grip morphs have a very strange idea of their own layoutFrame

Yes, grip morphs are quite hacky. Their relationship to SystemWindow and that window's label area required some unconventional overrides of #layoutProportionallyInBounds:(positioning:)

Anyway, that issue with #balanceOffsets: does not bleed into all these things. It is just confined within ProportionalSplitterMorph. I suppose. :-D

Yes, it appears I isolated balanceOffsets by the preference checks:

    (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ])

and in ProportionalSplitterMorph>>#stopStepping, it once again calls #balanceOffsets to put the state back to the manual-splitter scheme, as if the user had moved the bar themself.

 - Chris




Best,
Marcel

Am 17.11.2019 23:34:24 schrieb Nicolas Cellier <[hidden email]>:

Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...





Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

marcel.taeumel
Hi Nicolas.

how long does it take to produce such animation?

About 1-3 minutes if you know what to capture with Camtasia Studio on a Surface Pro 6 Laptop. 

Best,
Marcel

Am 20.11.2019 09:38:54 schrieb Nicolas Cellier <[hidden email]>:

Thanks Marcel!
how long does it take to produce such animation?

Chris, what is the purpose of fractional layouts?
It is to provide adaptive behavior to window resizing.
LayoutFrame enable having a mixture of fixed size widgets, variable size widgets with size proportional to window extent, or mixed case.
Patching the layout with absolute offsets, is a big mistake, because the layout then misbehave when window is resized, especially when shrinked.
The offsets are not proportional, and they are not meant to be, they are fixed width constraints!
Patching the offset also override the fixed width specification, which is another mistake: fixed size constraint information is lost.

What smart splitters do? They are introducing a new rule: size is proportional to contents.
It may express some constraints in term of absolute positioning/extent, that's not a problem.
What I deny is the fact that it would resolve those constraints by changing absolute offsets.
Smart splitters shall adapt the proportional layout of widgets, that's crystal clear.

The problem I exposed is not related to smart splitters, just to the way regular splitter (miss)-work.
We should first solve that, having a reasonable behavior and simple implementation, then we will come back to smart splitters.


Le mer. 20 nov. 2019 à 09:02, Marcel Taeumel <[hidden email]> a écrit :
This is the bug (smart splitters disabled, left mouse click):

Best,
Marcel

Am 20.11.2019 05:40:59 schrieb Chris Muller <[hidden email]>:

On Mon, Nov 18, 2019 at 2:20 AM Marcel Taeumel <[hidden email]> wrote:
Hi, there.

I cannot solve this issue at the moment.

Here is some history about (the rather recently added) #balanceOffsets:


one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds

That's exactly what's happening. See:

Morph >> #doLayoutIn:
ProportionalLayout >> #layoutIn:
Morph >> #layoutProportionallyInBounds:(positioning:)
LayoutFrame >> layout:in:

for example the grip morphs have a very strange idea of their own layoutFrame

Yes, grip morphs are quite hacky. Their relationship to SystemWindow and that window's label area required some unconventional overrides of #layoutProportionallyInBounds:(positioning:)

Anyway, that issue with #balanceOffsets: does not bleed into all these things. It is just confined within ProportionalSplitterMorph. I suppose. :-D

Yes, it appears I isolated balanceOffsets by the preference checks:

    (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ])

and in ProportionalSplitterMorph>>#stopStepping, it once again calls #balanceOffsets to put the state back to the manual-splitter scheme, as if the user had moved the bar themself.

 - Chris




Best,
Marcel

Am 17.11.2019 23:34:24 schrieb Nicolas Cellier <[hidden email]>:

Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...





Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

marcel.taeumel
Hi, all.

I just fixed this issue in Trunk (Morphic-mt.1590). Still, there is another, smaller, issue remaining:



Best,
Marcel

Am 20.11.2019 09:43:23 schrieb Marcel Taeumel <[hidden email]>:

Hi Nicolas.

how long does it take to produce such animation?

About 1-3 minutes if you know what to capture with Camtasia Studio on a Surface Pro 6 Laptop. 

Best,
Marcel

Am 20.11.2019 09:38:54 schrieb Nicolas Cellier <[hidden email]>:

Thanks Marcel!
how long does it take to produce such animation?

Chris, what is the purpose of fractional layouts?
It is to provide adaptive behavior to window resizing.
LayoutFrame enable having a mixture of fixed size widgets, variable size widgets with size proportional to window extent, or mixed case.
Patching the layout with absolute offsets, is a big mistake, because the layout then misbehave when window is resized, especially when shrinked.
The offsets are not proportional, and they are not meant to be, they are fixed width constraints!
Patching the offset also override the fixed width specification, which is another mistake: fixed size constraint information is lost.

What smart splitters do? They are introducing a new rule: size is proportional to contents.
It may express some constraints in term of absolute positioning/extent, that's not a problem.
What I deny is the fact that it would resolve those constraints by changing absolute offsets.
Smart splitters shall adapt the proportional layout of widgets, that's crystal clear.

The problem I exposed is not related to smart splitters, just to the way regular splitter (miss)-work.
We should first solve that, having a reasonable behavior and simple implementation, then we will come back to smart splitters.


Le mer. 20 nov. 2019 à 09:02, Marcel Taeumel <[hidden email]> a écrit :
This is the bug (smart splitters disabled, left mouse click):

Best,
Marcel

Am 20.11.2019 05:40:59 schrieb Chris Muller <[hidden email]>:

On Mon, Nov 18, 2019 at 2:20 AM Marcel Taeumel <[hidden email]> wrote:
Hi, there.

I cannot solve this issue at the moment.

Here is some history about (the rather recently added) #balanceOffsets:


one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds

That's exactly what's happening. See:

Morph >> #doLayoutIn:
ProportionalLayout >> #layoutIn:
Morph >> #layoutProportionallyInBounds:(positioning:)
LayoutFrame >> layout:in:

for example the grip morphs have a very strange idea of their own layoutFrame

Yes, grip morphs are quite hacky. Their relationship to SystemWindow and that window's label area required some unconventional overrides of #layoutProportionallyInBounds:(positioning:)

Anyway, that issue with #balanceOffsets: does not bleed into all these things. It is just confined within ProportionalSplitterMorph. I suppose. :-D

Yes, it appears I isolated balanceOffsets by the preference checks:

    (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ])

and in ProportionalSplitterMorph>>#stopStepping, it once again calls #balanceOffsets to put the state back to the manual-splitter scheme, as if the user had moved the bar themself.

 - Chris




Best,
Marcel

Am 17.11.2019 23:34:24 schrieb Nicolas Cellier <[hidden email]>:

Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...





Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

Nicolas Cellier
Hi Marcel,
unfortunately it did not solve the problem for me, but just displace the problem:
Open a MCPatchBrowser (with annotation panes preference enabled), click the splitter
and you will see it drifting downward...

Le mer. 20 nov. 2019 à 16:22, Marcel Taeumel <[hidden email]> a écrit :
Hi, all.

I just fixed this issue in Trunk (Morphic-mt.1590). Still, there is another, smaller, issue remaining:



Best,
Marcel

Am 20.11.2019 09:43:23 schrieb Marcel Taeumel <[hidden email]>:

Hi Nicolas.

how long does it take to produce such animation?

About 1-3 minutes if you know what to capture with Camtasia Studio on a Surface Pro 6 Laptop. 

Best,
Marcel

Am 20.11.2019 09:38:54 schrieb Nicolas Cellier <[hidden email]>:

Thanks Marcel!
how long does it take to produce such animation?

Chris, what is the purpose of fractional layouts?
It is to provide adaptive behavior to window resizing.
LayoutFrame enable having a mixture of fixed size widgets, variable size widgets with size proportional to window extent, or mixed case.
Patching the layout with absolute offsets, is a big mistake, because the layout then misbehave when window is resized, especially when shrinked.
The offsets are not proportional, and they are not meant to be, they are fixed width constraints!
Patching the offset also override the fixed width specification, which is another mistake: fixed size constraint information is lost.

What smart splitters do? They are introducing a new rule: size is proportional to contents.
It may express some constraints in term of absolute positioning/extent, that's not a problem.
What I deny is the fact that it would resolve those constraints by changing absolute offsets.
Smart splitters shall adapt the proportional layout of widgets, that's crystal clear.

The problem I exposed is not related to smart splitters, just to the way regular splitter (miss)-work.
We should first solve that, having a reasonable behavior and simple implementation, then we will come back to smart splitters.


Le mer. 20 nov. 2019 à 09:02, Marcel Taeumel <[hidden email]> a écrit :
This is the bug (smart splitters disabled, left mouse click):

Best,
Marcel

Am 20.11.2019 05:40:59 schrieb Chris Muller <[hidden email]>:

On Mon, Nov 18, 2019 at 2:20 AM Marcel Taeumel <[hidden email]> wrote:
Hi, there.

I cannot solve this issue at the moment.

Here is some history about (the rather recently added) #balanceOffsets:


one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds

That's exactly what's happening. See:

Morph >> #doLayoutIn:
ProportionalLayout >> #layoutIn:
Morph >> #layoutProportionallyInBounds:(positioning:)
LayoutFrame >> layout:in:

for example the grip morphs have a very strange idea of their own layoutFrame

Yes, grip morphs are quite hacky. Their relationship to SystemWindow and that window's label area required some unconventional overrides of #layoutProportionallyInBounds:(positioning:)

Anyway, that issue with #balanceOffsets: does not bleed into all these things. It is just confined within ProportionalSplitterMorph. I suppose. :-D

Yes, it appears I isolated balanceOffsets by the preference checks:

    (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ])

and in ProportionalSplitterMorph>>#stopStepping, it once again calls #balanceOffsets to put the state back to the manual-splitter scheme, as if the user had moved the bar themself.

 - Chris




Best,
Marcel

Am 17.11.2019 23:34:24 schrieb Nicolas Cellier <[hidden email]>:

Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...






Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

Chris Muller-3
In reply to this post by Nicolas Cellier
Hi Nicolas,

Chris, what is the purpose of fractional layouts?
It is to provide adaptive behavior to window resizing.

Right.
 
LayoutFrame enable having a mixture of fixed size widgets, variable size widgets with size proportional to window extent, or mixed case.
Patching the layout with absolute offsets, is a big mistake, because the layout then misbehave when window is resized, especially when shrinked.

It doesn't matter, the feature is about animating the splitters to keep the display of information constantly optimized.
 
The offsets are not proportional, and they are not meant to be, they are fixed width constraints!

Well, absolute offsets are used when simply dragging any splitter, right?  It's tied to the hand position.  That's probably the other problem you're seeing now -- some rounding glitch when calculating the new proportion with offset 0, after the drop.

Smart splitters is about realizing a finely-tuned IDE that meets certain explicit user requirements.  It uses Proportional whenever the user overrides with a manual drag, otherwise, the equivalent of "dragging splitters" is all only what it ever does, constantly, in response to content occlusion.

And, the feature is pretty much isolated off to the side, behind those two preferences, not hurting anyone.
 
Patching the offset also override the fixed width specification, which is another mistake: fixed size constraint information is lost. 

What smart splitters do? They are introducing a new rule: size is proportional to contents.
It may express some constraints in term of absolute positioning/extent, that's not a problem.
What I deny is the fact that it would resolve those constraints by changing absolute offsets. 
Smart splitters shall adapt the proportional layout of widgets, that's crystal clear.

The problem I exposed is not related to smart splitters, just to the way regular splitter (miss)-work.
We should first solve that, having a reasonable behavior and simple implementation, then we will come back to smart splitters.

I don't think the requirements demanded of smart-splitters can be met via proportional calculations.  There's too much variability and feedback.  The need for pixel-level refinements made it delicate and difficult to get optimized.  Maybe SOME improvement could be found somewhere but, trust me, messing with it will waste your time and drive you crazy.

Best,
  Chris



image.gif (900K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

marcel.taeumel
In reply to this post by Nicolas Cellier
Hi Nicolas,

hmmm.... it's funny that those few tools and tests we have can be sufficient to catch certain bugs quite comprehensively. :-D

Hopefully fixed that in Morphic-mt.1591.

Thanks for the pointer!

Note that the save dialog implements that fixed middle portion by moving it upwards (-47 pixel from top border) while the generic patch dialog moves it downward (+25 pixel from bottom border).

Further reading:

MCOperationsBrowser >> #widgetSpecs
... ((textMorph: annotations) (0 0.4 1 0.4) (0 0 0 defaultAnnotationPaneHeight "25")) ...
MCSaveVersionDialog >> #widgetSpecs
... ((textMorph: annotations) (0 0.6 0.5 0.6) (0 -47 0 0)) ...

Best,
Marcel

Am 20.11.2019 21:52:58 schrieb Nicolas Cellier <[hidden email]>:

Hi Marcel,
unfortunately it did not solve the problem for me, but just displace the problem:
Open a MCPatchBrowser (with annotation panes preference enabled), click the splitter
and you will see it drifting downward...

Le mer. 20 nov. 2019 à 16:22, Marcel Taeumel <[hidden email]> a écrit :
Hi, all.

I just fixed this issue in Trunk (Morphic-mt.1590). Still, there is another, smaller, issue remaining:



Best,
Marcel

Am 20.11.2019 09:43:23 schrieb Marcel Taeumel <[hidden email]>:

Hi Nicolas.

how long does it take to produce such animation?

About 1-3 minutes if you know what to capture with Camtasia Studio on a Surface Pro 6 Laptop. 

Best,
Marcel

Am 20.11.2019 09:38:54 schrieb Nicolas Cellier <[hidden email]>:

Thanks Marcel!
how long does it take to produce such animation?

Chris, what is the purpose of fractional layouts?
It is to provide adaptive behavior to window resizing.
LayoutFrame enable having a mixture of fixed size widgets, variable size widgets with size proportional to window extent, or mixed case.
Patching the layout with absolute offsets, is a big mistake, because the layout then misbehave when window is resized, especially when shrinked.
The offsets are not proportional, and they are not meant to be, they are fixed width constraints!
Patching the offset also override the fixed width specification, which is another mistake: fixed size constraint information is lost.

What smart splitters do? They are introducing a new rule: size is proportional to contents.
It may express some constraints in term of absolute positioning/extent, that's not a problem.
What I deny is the fact that it would resolve those constraints by changing absolute offsets.
Smart splitters shall adapt the proportional layout of widgets, that's crystal clear.

The problem I exposed is not related to smart splitters, just to the way regular splitter (miss)-work.
We should first solve that, having a reasonable behavior and simple implementation, then we will come back to smart splitters.


Le mer. 20 nov. 2019 à 09:02, Marcel Taeumel <[hidden email]> a écrit :
This is the bug (smart splitters disabled, left mouse click):

Best,
Marcel

Am 20.11.2019 05:40:59 schrieb Chris Muller <[hidden email]>:

On Mon, Nov 18, 2019 at 2:20 AM Marcel Taeumel <[hidden email]> wrote:
Hi, there.

I cannot solve this issue at the moment.

Here is some history about (the rather recently added) #balanceOffsets:


one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds

That's exactly what's happening. See:

Morph >> #doLayoutIn:
ProportionalLayout >> #layoutIn:
Morph >> #layoutProportionallyInBounds:(positioning:)
LayoutFrame >> layout:in:

for example the grip morphs have a very strange idea of their own layoutFrame

Yes, grip morphs are quite hacky. Their relationship to SystemWindow and that window's label area required some unconventional overrides of #layoutProportionallyInBounds:(positioning:)

Anyway, that issue with #balanceOffsets: does not bleed into all these things. It is just confined within ProportionalSplitterMorph. I suppose. :-D

Yes, it appears I isolated balanceOffsets by the preference checks:

    (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ])

and in ProportionalSplitterMorph>>#stopStepping, it once again calls #balanceOffsets to put the state back to the manual-splitter scheme, as if the user had moved the bar themself.

 - Chris




Best,
Marcel

Am 17.11.2019 23:34:24 schrieb Nicolas Cellier <[hidden email]>:

Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...


I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?

In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...






Reply | Threaded
Open this post in threaded view
|

Re: MCSaveVersionDialog splitter broken

Nicola Mingotti
In reply to this post by marcel.taeumel

Animated gif are a very good idea to show bugs in the mailing list ! 
I will keep it in mind.

bye
n.


On Nov 20, 2019, at 12:02 AM, Marcel Taeumel <[hidden email]> wrote:

This is the bug (smart splitters disabled, left mouse click):
<image.gif>

Best,
Marcel

Am 20.11.2019 05:40:59 schrieb Chris Muller <[hidden email]>:

On Mon, Nov 18, 2019 at 2:20 AM Marcel Taeumel <[hidden email]> wrote:
Hi, there.

I cannot solve this issue at the moment.

Here is some history about (the rather recently added) #balanceOffsets:


one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds

That's exactly what's happening. See:

Morph >> #doLayoutIn:
ProportionalLayout >> #layoutIn:
Morph >> #layoutProportionallyInBounds:(positioning:)
LayoutFrame >> layout:in:

for example the grip morphs have a very strange idea of their own layoutFrame

Yes, grip morphs are quite hacky. Their relationship to SystemWindow and that window's label area required some unconventional overrides of #layoutProportionallyInBounds:(positioning:)

Anyway, that issue with #balanceOffsets: does not bleed into all these things. It is just confined within ProportionalSplitterMorph. I suppose. :-D

Yes, it appears I isolated balanceOffsets by the preference checks:

    (ProportionalSplitterMorph smartVerticalSplitters or: [ ProportionalSplitterMorph smartHorizontalSplitters ])

and in ProportionalSplitterMorph>>#stopStepping, it once again calls #balanceOffsets to put the state back to the manual-splitter scheme, as if the user had moved the bar themself.

 - Chris




Best,
Marcel

Am 17.11.2019 23:34:24 schrieb Nicolas Cellier <[hidden email]>:

Hi,
what I see is completely against my opinion of the right thing.
I gonna expose this opinion below.

#repositionBy: is trying to change the layoutFrame offsets of the splitter and its neighbour morphs...
WHAT?

What are offsets for in the first place?
They are used for reserving a fixed space, for example for a button to be one lineGrid high!
A proportional splitter should not change the fixed space, it should rather consider it as a constraint.
Instead, it should change the proportional space, that is the fraction part of layouts...
As the name tells! A ProportionalSplitter SHALL change the proportional layout.

What is #balanceOffsets trying to do?
It's trying to undo the mistakes that we performed in #repositionBy:by transforming the offset deltas into fraction deltas...
Ouch, it's too late, I don't feel like this is a bright strategy!

By inspecting the Morphs, I see plenty of surprises... For example, one could expect that a Morph bounds is obtained by applying its layoutFrame to its owner bounds... Err, that ain't always the case, for example the grip morphs have a very strange idea of their own layoutFrame... So before I learn what the least surprise should be, it might take a bit of time... Otherwise, repairing one thing may break two others...
If you have already gone thru this, share my opinion, and are ready to kill the problem you get my blessing ;)

Le dim. 17 nov. 2019 à 20:41, Nicolas Cellier <[hidden email]> a écrit :
Bingo,
if I inspect the ProportionalSpliiterMorph (thru halos), then evaluate in inspector:

    self setProperty: #fullDelta toValue: 0@0; balanceOffsets

I then get the undesired behavior...
Thanks Christoph for the starting point

Le dim. 17 nov. 2019 à 19:20, Thiede, Christoph <[hidden email]> a écrit :

Hi, I perceived a similar issue a year ago, I'm afraid it appears to have been forgotten ...



I recently found a bug in the splitter class: Each time you mouseDown a splitter but not mouseMove it, it moves itself by delta=1. More concretely I could locate this during debugging in #balanceOffsets, where self layoutFrame leftFraction actually gets a larger value with every click. I was able to fix the problem by preventing every call to #balanceOffsets, but that doesn't seem to be the right solution. Since then, I haven't missed any splitter behavior, so I don't understand the point of this method.

Is there any reason why #fullDelta is set here?


In every case, I think it could be a good idea to refactor #balanceOffsets in terms of readability :-)

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Nicolas Cellier <[hidden email]>
Gesendet: Sonntag, 17. November 2019 19:11 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] MCSaveVersionDialog splitter broken
 
Hi,
here is how to reproduce:
1. save any package
2. click on the horizontal splitter (below the list of changes)
3. the splitter goes up once at button down, and once at button up by about the vertical size of the Accept/Cancel buttons

I don't know where to start searching for it...