Roassal TRCompositeShape regression

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

Roassal TRCompositeShape regression

Peter Uhnak
Hi Alex,

unfortunately changes in TRCompositeShape created a regression, because now TRRemoveCallbacks are executed multiple times.

the commit is Trachel-AlexandreBergel.266

Interestingly there is TRCompositeShapeTest>>testCallback01 which works, but it doesn't use RTElement.

Here's a test case:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|v e executed|
v := RTView new.
e := RTElement new.
e + (RTBox new) + (RTEllipse new).

executed := 0.
e trachelShape addCallback: (TRRemoveCallback new block: [ :shape | executed := executed + 1 ]).

v add: e.

TestAsserter new assert: executed equals: 0.
e remove.
TestAsserter new assert: executed equals: 1.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Now since you have discussed with Jan that the new behavior makes sense in some circumstances, it would be better to have both behaviors.

So I would propose to make a distinction for RemoveCallback - to not triggered them from TRCompositeShape, because it is not reentrant.

Thus when executed on composite shape, only non-removing callbacks would be executed.

Additionally it seems very random that CompositeShape uses it's first subshape for callbacks; what is so special about the first one? Why can't composite shape hold it's own callbacks, or have an explicit way to specify which one to use (first, second, none, ..)?

Since Composite Shape is recurring problematic theme, perhaps there should be some wider discussion to collect all the use cases?

What do you think?

Peter

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

Peter Uhnak
Bumping so this doesn't get lost.

Also would it be possible to at least make a release with Trachel-265 (before the breaking commit) so I can freeze the dependency before it gets fixed?

Or is there other way I can directly specify what commit will load in my ConfigurationOf?

Thanks,
Peter

From: [hidden email]
Sent: ‎4/‎1/‎2015 7:13 PM
To: [hidden email]
Subject: Roassal TRCompositeShape regression

Hi Alex,

unfortunately changes in TRCompositeShape created a regression, because now TRRemoveCallbacks are executed multiple times.

the commit is Trachel-AlexandreBergel.266

Interestingly there is TRCompositeShapeTest>>testCallback01 which works, but it doesn't use RTElement.

Here's a test case:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|v e executed|
v := RTView new.
e := RTElement new.
e + (RTBox new) + (RTEllipse new).

executed := 0.
e trachelShape addCallback: (TRRemoveCallback new block: [ :shape | executed := executed + 1 ]).

v add: e.

TestAsserter new assert: executed equals: 0.
e remove.
TestAsserter new assert: executed equals: 1.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Now since you have discussed with Jan that the new behavior makes sense in some circumstances, it would be better to have both behaviors.

So I would propose to make a distinction for RemoveCallback - to not triggered them from TRCompositeShape, because it is not reentrant.

Thus when executed on composite shape, only non-removing callbacks would be executed.

Additionally it seems very random that CompositeShape uses it's first subshape for callbacks; what is so special about the first one? Why can't composite shape hold it's own callbacks, or have an explicit way to specify which one to use (first, second, none, ..)?

Since Composite Shape is recurring problematic theme, perhaps there should be some wider discussion to collect all the use cases?

What do you think?

Peter

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

abergel
In reply to this post by Peter Uhnak
Let me check this…
Indeed, it would be great to have a deep discussion about composite shapes.

Will you be at ESUG?

Cheers,
Alexandre


> On Apr 1, 2015, at 2:13 PM, Peter Uhnák <[hidden email]> wrote:
>
> Hi Alex,
>
> unfortunately changes in TRCompositeShape created a regression, because now TRRemoveCallbacks are executed multiple times.
>
> the commit is Trachel-AlexandreBergel.266
>
> Interestingly there is TRCompositeShapeTest>>testCallback01 which works, but it doesn't use RTElement.
>
> Here's a test case:
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> |v e executed|
> v := RTView new.
> e := RTElement new.
> e + (RTBox new) + (RTEllipse new).
>
> executed := 0.
> e trachelShape addCallback: (TRRemoveCallback new block: [ :shape | executed := executed + 1 ]).
>
> v add: e.
>
> TestAsserter new assert: executed equals: 0.
> e remove.
> TestAsserter new assert: executed equals: 1.
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Now since you have discussed with Jan that the new behavior makes sense in some circumstances, it would be better to have both behaviors.
>
> So I would propose to make a distinction for RemoveCallback - to not triggered them from TRCompositeShape, because it is not reentrant.
>
> Thus when executed on composite shape, only non-removing callbacks would be executed.
>
> Additionally it seems very random that CompositeShape uses it's first subshape for callbacks; what is so special about the first one? Why can't composite shape hold it's own callbacks, or have an explicit way to specify which one to use (first, second, none, ..)?
>
> Since Composite Shape is recurring problematic theme, perhaps there should be some wider discussion to collect all the use cases?
>
> What do you think?
>
> Peter
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev

--
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.




_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

abergel
In reply to this post by Peter Uhnak
Bumping so this doesn't get lost.

Also would it be possible to at least make a release with Trachel-265 (before the breaking commit) so I can freeze the dependency before it gets fixed?

Or is there other way I can directly specify what commit will load in my ConfigurationOf?

I have produced version 1.52 of Roassal. ConfigurationOfRoassal2 in the repo Roassal2 and MetaRepoPharo4.0

But best is to solve the Trachel issue with the callback.

Alexandre



Thanks,
Peter

From: [hidden email]
Sent: ‎4/‎1/‎2015 7:13 PM
To: [hidden email]
Subject: Roassal TRCompositeShape regression

Hi Alex,

unfortunately changes in TRCompositeShape created a regression, because now TRRemoveCallbacks are executed multiple times.

the commit is Trachel-AlexandreBergel.266

Interestingly there is TRCompositeShapeTest>>testCallback01 which works, but it doesn't use RTElement.

Here's a test case:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|v e executed|
v := RTView new.
e := RTElement new.
e + (RTBox new) + (RTEllipse new).

executed := 0.
e trachelShape addCallback: (TRRemoveCallback new block: [ :shape | executed := executed + 1 ]).

v add: e.

TestAsserter new assert: executed equals: 0.
e remove.
TestAsserter new assert: executed equals: 1.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Now since you have discussed with Jan that the new behavior makes sense in some circumstances, it would be better to have both behaviors.

So I would propose to make a distinction for RemoveCallback - to not triggered them from TRCompositeShape, because it is not reentrant.

Thus when executed on composite shape, only non-removing callbacks would be executed.

Additionally it seems very random that CompositeShape uses it's first subshape for callbacks; what is so special about the first one? Why can't composite shape hold it's own callbacks, or have an explicit way to specify which one to use (first, second, none, ..)?

Since Composite Shape is recurring problematic theme, perhaps there should be some wider discussion to collect all the use cases?

What do you think?

Peter
_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

Peter Uhnak
Will you be at ESUG?
I'm not sure yet, it depends more on my university than me... but hopefully I will know soon.

I have produced version 1.52 of Roassal. ConfigurationOfRoassal2 in the repo Roassal2 and MetaRepoPharo4.0

Thanks, but would it be possible to also include Roassal2Spec-johanfabry.6? This would probably be needed in all versions, because otherwise one cannot use Roassal2Spec in latest Pharo.

But best is to solve the Trachel issue with the callback.
I know, but I need more stable environment (I can easily deal with it, but when other people are working with it and are not necessarily devs, I need a more stable environment for them).

I have some ideas about having more free composite shape, where I could specify (either with blocks, or having the option to subclass CompositeShape) what kind of behavior I want, but it definitely needs more refining. Because recently I ran into issue that when resizing a composite shape I wanted to resize only two of them (borders) but not another two (icon and label), so I had to abandon CompositeShape for that problem and do it manually.

Thanks,
Peter

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

abergel

Will you be at ESUG?
I'm not sure yet, it depends more on my university than me... but hopefully I will know soon.

Ok. I still haven’t my flight ticket. But it is likely that I will attend.
It would be great to meet!


I have produced version 1.52 of Roassal. ConfigurationOfRoassal2 in the repo Roassal2 and MetaRepoPharo4.0

Thanks, but would it be possible to also include Roassal2Spec-johanfabry.6? This would probably be needed in all versions, because otherwise one cannot use Roassal2Spec in latest Pharo.

Done!


But best is to solve the Trachel issue with the callback.
I know, but I need more stable environment (I can easily deal with it, but when other people are working with it and are not necessarily devs, I need a more stable environment for them).

Yes!

I have some ideas about having more free composite shape, where I could specify (either with blocks, or having the option to subclass CompositeShape) what kind of behavior I want, but it definitely needs more refining. Because recently I ran into issue that when resizing a composite shape I wanted to resize only two of them (borders) but not another two (icon and label), so I had to abandon CompositeShape for that problem and do it manually.

Ah ah! You are facing some of the trouble I have already experienced with composed shapes. This is indeed a serious beast. Welcome on board!!!

Alexandre


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

Jan Blizničenko
Hi

I'd like to break into your discussion, so we won't talk about the same in two threads.

Composite shapes should, to outer world, act line normal shapes, which could be moved, resized, used callbacks on and they should act like we expect usual shapes to. Thing is it is not quite clear what is this "expected" behavior and in my opinion it depends on actual usage of composite shapes, especially with resizing and callbacks.

One might want to resize only one border-like shape and keep rest as it is, or to resize all subshapes to new size, or resize all proportionally, like increase width by 50 %.

The same is there for callbacks, when we might want to have callback for every change of extent of any inner shape, or we might want to have callback only when one of them changes extent, or only when whole shape changes extent (because when we change extent of shape in the center of larger one, it does not change extent of whole shape).

I thought about using replacable blocks for composite shapes actions like adding callbacks, resizing or moving composite shape, reaction to moving or resizing sub-shapes etc.

Jan

abergel wrote
> But best is to solve the Trachel issue with the callback.
> I know, but I need more stable environment (I can easily deal with it, but when other people are working with it and are not necessarily devs, I need a more stable environment for them).

Yes!

> I have some ideas about having more free composite shape, where I could specify (either with blocks, or having the option to subclass CompositeShape) what kind of behavior I want, but it definitely needs more refining. Because recently I ran into issue that when resizing a composite shape I wanted to resize only two of them (borders) but not another two (icon and label), so I had to abandon CompositeShape for that problem and do it manually.

Ah ah! You are facing some of the trouble I have already experienced with composed shapes. This is indeed a serious beast. Welcome on board!!!

Alexandre


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

Peter Uhnak
In reply to this post by abergel
Hi Alex,
 
Will you be at ESUG?
I'm not sure yet, it depends more on my university than me... but hopefully I will know soon.

Ok. I still haven’t my flight ticket. But it is likely that I will attend.
It would be great to meet!

I will be at ESUG, so I'm looking forward to meeting you. :)

Peter

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

abergel
Excellent!!!

Alexandre


> On Apr 12, 2015, at 4:51 PM, Peter Uhnák <[hidden email]> wrote:
>
> Hi Alex,
>  
>> Will you be at ESUG?
>> I'm not sure yet, it depends more on my university than me... but hopefully I will know soon.
>
> Ok. I still haven’t my flight ticket. But it is likely that I will attend.
> It would be great to meet!
>
> I will be at ESUG, so I'm looking forward to meeting you. :)
>
> Peter
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev

--
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.




_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

abergel
In reply to this post by Jan Blizničenko
Hi Jan,

I share the same feeling than yours.
At the very beginning, shapes was supposed to act as a normal shapes. But then, particular requirements appears such as resize. I guess soon we will have other requirements such as adding inner shapes dynamically.

So far, I have essentially used the composite shapes to insert labels. Maybe this is what the composite shapes should be good at. And no more than that.

Let’s meet at esug for a live discussion.

Alexandre



> On Apr 8, 2015, at 3:10 PM, Jan B. <[hidden email]> wrote:
>
> Hi
>
> I'd like to break into your discussion, so we won't talk about the same in
> two threads.
>
> Composite shapes should, to outer world, act line normal shapes, which could
> be moved, resized, used callbacks on and they should act like we expect
> usual shapes to. Thing is it is not quite clear what is this "expected"
> behavior and in my opinion it depends on actual usage of composite shapes,
> especially with resizing and callbacks.
>
> One might want to resize only one border-like shape and keep rest as it is,
> or to resize all subshapes to new size, or resize all proportionally, like
> increase width by 50 %.
>
> The same is there for callbacks, when we might want to have callback for
> every change of extent of any inner shape, or we might want to have callback
> only when one of them changes extent, or only when whole shape changes
> extent (because when we change extent of shape in the center of larger one,
> it does not change extent of whole shape).
>
> I thought about using replacable blocks for composite shapes actions like
> adding callbacks, resizing or moving composite shape, reaction to moving or
> resizing sub-shapes etc.
>
> Jan
>
>
> abergel wrote
>>> But best is to solve the Trachel issue with the callback.
>>> I know, but I need more stable environment (I can easily deal with it,
>>> but when other people are working with it and are not necessarily devs, I
>>> need a more stable environment for them).
>>
>> Yes!
>>
>>> I have some ideas about having more free composite shape, where I could
>>> specify (either with blocks, or having the option to subclass
>>> CompositeShape) what kind of behavior I want, but it definitely needs
>>> more refining. Because recently I ran into issue that when resizing a
>>> composite shape I wanted to resize only two of them (borders) but not
>>> another two (icon and label), so I had to abandon CompositeShape for that
>>> problem and do it manually.
>>
>> Ah ah! You are facing some of the trouble I have already experienced with
>> composed shapes. This is indeed a serious beast. Welcome on board!!!
>>
>> Alexandre
>>
>>
>> _______________________________________________
>> Moose-dev mailing list
>
>> Moose-dev@.unibe
>
>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
>
>
>
>
> --
> View this message in context: http://forum.world.st/Roassal-TRCompositeShape-regression-tp4816726p4818430.html
> Sent from the Moose mailing list archive at Nabble.com.
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev

--
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.




_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

Peter Uhnak
I guess soon we will have other requirements such as adding inner shapes dynamically.
We already do this. :) (and of course it is draggable and resizable, both the outer and inner elements :))

But since we have an extra layer on top of Roassal we can somewhat offset this problem — each shape has a controller, where we define the necessary behavior; however this is really an abuse of the controller as it has different responsibilities. And if there is no best way or systematic structure than it is always reimplemented manually (of course everywhere differently), so it is a constantly increasing mess... but this will definitely require more thinking and designing to get this whole problem right.

Peter

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

abergel
This is pushing inner shapes to its limit I have the impression.

Alexandre
-- 
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.



On Apr 12, 2015, at 5:43 PM, Peter Uhnák <[hidden email]> wrote:

I guess soon we will have other requirements such as adding inner shapes dynamically.
We already do this. :) (and of course it is draggable and resizable, both the outer and inner elements :))

But since we have an extra layer on top of Roassal we can somewhat offset this problem — each shape has a controller, where we define the necessary behavior; however this is really an abuse of the controller as it has different responsibilities. And if there is no best way or systematic structure than it is always reimplemented manually (of course everywhere differently), so it is a constantly increasing mess... but this will definitely require more thinking and designing to get this whole problem right.

Peter
_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

Peter Uhnak

This is pushing inner shapes to its limit I have the impression.

Well yesterday I had to add new method to TRMorph because I was changing dragged shape on drag start... so I would say we are well beyond the limits.
But at least there will be more things to discuss at ESUG. :)
(And more work for me to clean it all up so it can be integrated into Roassal.)

Peter

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

abergel
Yes!

if you are planning significant changes, best would be to discuss before

Alexandre
-- 
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.



On Apr 15, 2015, at 2:03 PM, Peter Uhnák <[hidden email]> wrote:


This is pushing inner shapes to its limit I have the impression.

Well yesterday I had to add new method to TRMorph because I was changing dragged shape on drag start... so I would say we are well beyond the limits.
But at least there will be more things to discuss at ESUG. :)
(And more work for me to clean it all up so it can be integrated into Roassal.)

Peter
_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

Jan Blizničenko
Hi once again

I'd like to bring here together all current problems of composite shapes, add one more and propose one fix.


Problem 1)
This topic was started because remove callbacks got triggered from same shape multiple times by single event. I think I fixed this problem. And I did few more things to TRCompositeShape while I was at it. I fixed exactly same problem with translation callbacks, I modified composite shape callback test to test this case with removing, and I added and used shapesDo: and firstShape methods to simplify code and make it faster - calling Collection>>#collect:thenDo: is faster than (collect:)do:, because is does not create temporary collection in the middle of it.
mcz: http://www.mediafire.com/download/lh37m9n0rq21e7n/Trachel-bliznjan.275.zip


Problem 2)
Unfortunately, in the meantime, I found much nastier bug, which is not so common to trigger, but it causes infinite loop and it happens in our DynaCASE app.
It happens only if:
We have a composite shape, canvas of this composite shape is not set (is nil), and then we add TRTranslationCallback to that shape - callback which gets encompassingRectangle of this shape in its block.

If canvas is nil, method encompassingRectangle sends (for some reason unclear to me) positionShapesAfterBeingAdded, which translates subshapes, which triggers translation callback, which sends encompassingRectangle, which sends positionShapesAfterBeingAdded etc...

Same problem happens by calling many methods which then call encompassingRectanhle, like center, position, extent, height and width.

Example code:
| shape process |
shape := (RTBox new + RTBox new) element.
shape addCallback: (TRTranslationCallback block: [shape encompassingRectangle]).
process := [shape translateTo: 100@100] fork.
(Delay forMilliseconds: 300) wait.
process suspend; debug.


Problem 3)
As we discussed in topic "[Roassal] TRCompositeShape not sending callbacks" ( http://forum.world.st/Roassal-TRCompositeShape-not-sending-callbacks-td4814221.html ), there is a problem if I use simple addCallback: for translation or extent callbacks, because this way it triggers only when first subshape is changed, but after that, the rest of subshapes get changed, but no callback is triggered anymore. You proposed method addCallbackToAllShapes:, but it would require to make nasty exceptions and type checking for composite shapes in multiple places in our code, because TRRemoveCallbacks have to be triggered only once per composite shape (which seems to be opposite of translation and extent callbacks).

Only possible solution that occurs to me is to edit addCallback: just for our need in subclass of RTCompositeShape, or use mentioned replacable blocks for that in TRCompositeShape.


Question 1 (maybe Problem 4?))
And to finish this, I have one question. What is and/or should be difference between TRShape>>#position and TRShape>>#center? In case of RTElement, it is explicitely the same, but what about here, where it sometimes is and sometimes isn't the same, especially with composite shapes (but not only them)?


Jan Blizničenko
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

abergel
Hi Jan!

First of all, sorry for replaying a bit slowly. My week end was quite busy and I was not in front of my computer.


Problem 1)
This topic was started because remove callbacks got triggered from same
shape multiple times by single event. I think I fixed this problem. And I
did few more things to TRCompositeShape while I was at it. I fixed exactly
same problem with translation callbacks, I modified composite shape callback
test to test this case with removing, and I added and used shapesDo: and
firstShape methods to simplify code and make it faster - calling
Collection>>#collect:thenDo: is faster than (collect:)do:, because is does
not create temporary collection in the middle of it.
mcz:
http://www.mediafire.com/download/lh37m9n0rq21e7n/Trachel-bliznjan.275.zip

It is integrated in Roassal!
Thanks!

Problem 2)
Unfortunately, in the meantime, I found much nastier bug, which is not so
common to trigger, but it causes infinite loop and it happens in our
DynaCASE app.
It happens only if:
We have a composite shape, canvas of this composite shape is not set (is
nil), and then we add TRTranslationCallback to that shape - callback which
gets encompassingRectangle of this shape in its block.

If canvas is nil, method encompassingRectangle sends (for some reason
unclear to me) positionShapesAfterBeingAdded, which translates subshapes,
which triggers translation callback, which sends encompassingRectangle,
which sends positionShapesAfterBeingAdded etc...

Same problem happens by calling many methods which then call
encompassingRectanhle, like center, position, extent, height and width.

Example code:
| shape process |
shape := (RTBox new + RTBox new) element.
shape addCallback: (TRTranslationCallback block: [shape
encompassingRectangle]).
process := [shape translateTo: 100@100] fork.
(Delay forMilliseconds: 300) wait.
process suspend; debug.

Hum… This is a strange bug…

I do not quite understand what’s going on here. I have an error in 

TRCompositeShape>>shapes 
^ shapeAndOffsets collect: #first

Problem 3)
As we discussed in topic "[Roassal] TRCompositeShape not sending callbacks"
(
http://forum.world.st/Roassal-TRCompositeShape-not-sending-callbacks-td4814221.html
), there is a problem if I use simple addCallback: for translation or extent
callbacks, because this way it triggers only when first subshape is changed,
but after that, the rest of subshapes get changed, but no callback is
triggered anymore. You proposed method addCallbackToAllShapes:, but it would
require to make nasty exceptions and type checking for composite shapes in
multiple places in our code, because TRRemoveCallbacks have to be triggered
only once per composite shape (which seems to be opposite of translation and
extent callbacks).

Only possible solution that occurs to me is to edit addCallback: just for
our need in subclass of RTCompositeShape, or use mentioned replacable blocks
for that in TRCompositeShape.

Sincerely, I am not convinced at all by TRCompositeShape, as it is implemented now. 
For example, having a shapeAndOffsets variable is odd to me.

Apparently, you guys are having a deeper use of it than I do. You can easily make your own CompositeShape, and all in a modular fashion, without  touching the core of either roassal or trachel. 

I would like to help you on this, but I am not sure how.


Question 1 (maybe Problem 4?))
And to finish this, I have one question. What is and/or should be difference
between TRShape>>#position and TRShape>>#center? In case of RTElement, it is
explicitely the same, but what about here, where it sometimes is and
sometimes isn't the same, especially with composite shapes (but not only
them)?

I think this is because of the composite shape. A question: can a composite shape have a position that is different than its center? I think yes.

Cheers,
Alexandre




Jan Blizničenko



--
View this message in context: http://forum.world.st/Roassal-TRCompositeShape-regression-tp4816726p4820464.html
Sent from the Moose mailing list archive at Nabble.com.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Roassal TRCompositeShape regression

Peter Uhnak
I think this is because of the composite shape. A question: can a composite shape have a position that is different than its center? I think yes.
 
I do not think that using anything else besides center is a good idea. There are many places (e.g. TRConstraint) that counts on the fact that position is a center. Other option would be to always return topLeft corner, however the most important part that it is always the same. If you wouldn't know what the position refers to than you can hardly use it.

Peter

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev