SCCompositeView bug

View: New views
6 Messages — Rating Filter:   Alert me  

SCCompositeView bug

by Scott Wilson-8 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Dig it:

// works fine
(
w = SCWindow.new(bounds: Rect(100, 100, 1000, 500)).front;

b = SCScrollView(w, Rect(0,0,1000, 500));
x = 14;
c = SCHLayoutView(b, Rect(0, 50, x * 104, 500));

x.do({SCSlider(c, Rect(0, 0, 100, 400)).background_(Color.black)});
)

// broken by rev 7297
(
w = SCWindow.new(bounds: Rect(100, 100, 1000, 500)).front;

b = SCScrollView(w, Rect(0,0,1000, 500));
x = 14;
c = SCHLayoutView(b, Rect(0, 50, x * 104, 500));

x.do({SCCompositeView(c, Rect(0, 0, 100, 400)).background_(Color.black)});
)

Prior to rev 7297 this worked correctly. Commit note:

fixing: putting a composite view inside a composite view, getting the correct absolute bounds.
adding: an absolute bounds getter method to SCView

Perhaps the getter is not working right in this special case?

S.

Re: SCCompositeView bug

by Scott Wilson-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Idly messing around, if I change this:

SCContainerView::SCContainerView(SCContainerView *inParent, PyrObject* inObj, SCRect inBounds)
: SCView(inParent, inObj, inBounds), mChildren(0), mNumChildren(0), mRelativeOrigin(false)
mLayout.bounds = inBounds;

back to this:

mLayout.bounds = mBounds;

which is what it was before, it all works again.

But tracing this through is giving me a headache. (When will SCCompositeView die...)

Felix this was your commit, do you recall why it was changed?

SCCompositeView.test still passes after I've changed this, and the absolute bounds seem to report correctly in my example.

S.

On 17 Jul 2008, at 16:51, Scott Wilson wrote:

Dig it:

// works fine
(
w = SCWindow.new(bounds: Rect(100, 100, 1000, 500)).front;

b = SCScrollView(w, Rect(0,0,1000, 500));
x = 14;
c = SCHLayoutView(b, Rect(0, 50, x * 104, 500));

x.do({SCSlider(c, Rect(0, 0, 100, 400)).background_(Color.black)});
)

// broken by rev 7297
(
w = SCWindow.new(bounds: Rect(100, 100, 1000, 500)).front;

b = SCScrollView(w, Rect(0,0,1000, 500));
x = 14;
c = SCHLayoutView(b, Rect(0, 50, x * 104, 500));

x.do({SCCompositeView(c, Rect(0, 0, 100, 400)).background_(Color.black)});
)

Prior to rev 7297 this worked correctly. Commit note:

fixing: putting a composite view inside a composite view, getting the correct absolute bounds.
adding: an absolute bounds getter method to SCView

Perhaps the getter is not working right in this special case?

S.


Re: SCCompositeView bug

by felix-38 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


hmm...  at the time it appeared to fix the problem.

I can confirm: changing it back, the unit tests still pass.

and the one you post works correctly.

all of my scrolls still work.

the example you posted strangly gets the abs bounds correct:

Rect(0, 50, 100, 500)

Rect(104, 50, 100, 500)

Rect(208, 50, 100, 500)

...

but it draws them wrong.

the SCScrollView unit test still has a failure on both.
but I think its harmless.  it has to do with bounds not updating during the same computation cycle as the drawing.

so I would tentatively say commit the change back. 

I think we should rename mBounds to mAbsBounds (is it ?)
and...all of these.
all these bounds vars need to be more explicitly named. 
that would really help to follow that code.



On Thu, Jul 17, 2008 at 8:39 PM, Scott Wilson <s.d.wilson.1@...> wrote:
Idly messing around, if I change this:

SCContainerView::SCContainerView(SCContainerView *inParent, PyrObject* inObj, SCRect inBounds)
: SCView(inParent, inObj, inBounds), mChildren(0), mNumChildren(0), mRelativeOrigin(false)
mLayout.bounds = inBounds;

back to this:

mLayout.bounds = mBounds;

which is what it was before, it all works again.

But tracing this through is giving me a headache. (When will SCCompositeView die...)

Felix this was your commit, do you recall why it was changed?

SCCompositeView.test still passes after I've changed this, and the absolute bounds seem to report correctly in my example.

S.

On 17 Jul 2008, at 16:51, Scott Wilson wrote:

Dig it:

// works fine
(
w = SCWindow.new(bounds: Rect(100, 100, 1000, 500)).front;

b = SCScrollView(w, Rect(0,0,1000, 500));
x = 14;
c = SCHLayoutView(b, Rect(0, 50, x * 104, 500));

x.do({SCSlider(c, Rect(0, 0, 100, 400)).background_(Color.black)});
)

// broken by rev 7297
(
w = SCWindow.new(bounds: Rect(100, 100, 1000, 500)).front;

b = SCScrollView(w, Rect(0,0,1000, 500));
x = 14;
c = SCHLayoutView(b, Rect(0, 50, x * 104, 500));

x.do({SCCompositeView(c, Rect(0, 0, 100, 400)).background_(Color.black)});
)

Prior to rev 7297 this worked correctly. Commit note:

fixing: putting a composite view inside a composite view, getting the correct absolute bounds.
adding: an absolute bounds getter method to SCView

Perhaps the getter is not working right in this special case?

S.



Re: SCCompositeView bug

by Scott Wilson-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


On 17 Jul 2008, at 22:24, felix wrote:


hmm...  at the time it appeared to fix the problem.

I can confirm: changing it back, the unit tests still pass.

and the one you post works correctly.

all of my scrolls still work.

the example you posted strangly gets the abs bounds correct:

Rect(0, 50, 100, 500)

Rect(104, 50, 100, 500)

Rect(208, 50, 100, 500)

...

but it draws them wrong.

Before the fix the last add composite view has these absolute bounds:

Rect(0, 0, 100, 400)

which is actually where it draws. I have no idea why that happens. The others are correct.


the SCScrollView unit test still has a failure on both.
but I think its harmless.  it has to do with bounds not updating during the same computation cycle as the drawing.

Sorry what's the failure? Here it says everything passed and I think everything looks okay.



so I would tentatively say commit the change back. 

I think we should rename mBounds to mAbsBounds (is it ?)

Not 100% sure. There may be exceptions. Ack!

and...all of these.
all these bounds vars need to be more explicitly named. 
that would really help to follow that code.

Yes it's extremely convoluted by this point. It's pretty close to working now though, and now that we have good container and scroll views hopefully there won't be anything too much to muck up the works.

S.





On Thu, Jul 17, 2008 at 8:39 PM, Scott Wilson <s.d.wilson.1@...> wrote:
Idly messing around, if I change this:

SCContainerView::SCContainerView(SCContainerView *inParent, PyrObject* inObj, SCRect inBounds)
: SCView(inParent, inObj, inBounds), mChildren(0), mNumChildren(0), mRelativeOrigin(false)
mLayout.bounds = inBounds;

back to this:

mLayout.bounds = mBounds;

which is what it was before, it all works again.

But tracing this through is giving me a headache. (When will SCCompositeView die...)

Felix this was your commit, do you recall why it was changed?

SCCompositeView.test still passes after I've changed this, and the absolute bounds seem to report correctly in my example.

S.

On 17 Jul 2008, at 16:51, Scott Wilson wrote:

Dig it:

// works fine
(
w = SCWindow.new(bounds: Rect(100, 100, 1000, 500)).front;

b = SCScrollView(w, Rect(0,0,1000, 500));
x = 14;
c = SCHLayoutView(b, Rect(0, 50, x * 104, 500));

x.do({SCSlider(c, Rect(0, 0, 100, 400)).background_(Color.black)});
)

// broken by rev 7297
(
w = SCWindow.new(bounds: Rect(100, 100, 1000, 500)).front;

b = SCScrollView(w, Rect(0,0,1000, 500));
x = 14;
c = SCHLayoutView(b, Rect(0, 50, x * 104, 500));

x.do({SCCompositeView(c, Rect(0, 0, 100, 400)).background_(Color.black)});
)

Prior to rev 7297 this worked correctly. Commit note:

fixing: putting a composite view inside a composite view, getting the correct absolute bounds.
adding: an absolute bounds getter method to SCView

Perhaps the getter is not working right in this special case?

S.




Re: SCCompositeView bug

by felix-38 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

I only just checked in the one test with the failure.  its not important tho.



Re: SCCompositeView bug

by Jan Trutzschler-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

mBounds should be the absolute position indeed and there shouldn't be  
any exceptions. i'll double check though ..

On Jul 17, 2008, at 11:44 PM, Scott Wilson wrote:

>>
>> I think we should rename mBounds to mAbsBounds (is it ?)
>
> Not 100% sure. There may be exceptions. Ack!


_______________________________________________
sc-dev mailing list

info (subscribe and unsubscribe): http://swiki.hfbk-hamburg.de:8888/MusicTechnology/880
archive: http://www.listarc.bham.ac.uk/marchives/sc-dev/
search: http://www.listarc.bham.ac.uk/lists/sc-dev/search/
LightInTheBox - Buy quality products at wholesale price