piccolo2d / piccolo2d.java

Structured 2D Graphics Framework
http://piccolo2d.org
Other
51 stars 14 forks source link

If width or height is zero, the rest of bounds are reset. #155

Closed mro closed 9 years ago

mro commented 9 years ago

Originally reported on Google Code with ID 155

What steps will reproduce the problem?

1. Here is some code to reproduce the issue:

        PNode testNode3 = new PNode();
        testNode3.setBounds(10, 10, 10, 0);
        System.out.println("Bounds for plain PNode = " + 
testNode3.getFullBoundsReference());

What is the expected output? What do you see instead?

I would expect that the bounds would be reported as 10, 10, 10, 0, but 
instead they are reported as being empty.

What version of the product are you using? On what operating system?

We are using version 1.2.1, but we have inspected the source code and the 
issue still appears to exist in trunk, r922.  OS is Windows XP.

Please provide any additional information below.

The reason that this is an issue for us is that in some cases we are 
laying out graphs with a number of PNodes whose positions depend on each 
other.  Setting the bounds to have only a width or height is necessary in 
some cases where the node (such as a textual label) has no value in some 
contexts, yet we still want to do a layout based on its position.  
However, the x and y values of the bounds are reset if the width or height 
are 0, so such a layout fails.  It is possible to work around this by 
detecting an empty bounds and using another PNode for the layout 
reference, but it makes the code much more complex.

We have no idea if other portions of the Piccolo library depend on the 
current behavior of resetting the bounds when length or height are 0.

Regards,
John Blanco
PhET Project
University of Colorado

Reported by jxb147 on 2010-01-14 19:27:10

mro commented 9 years ago
Thank you for reporting this.

Reported by allain.lalonde on 2010-01-14 19:44:25

mro commented 9 years ago
Fixed in r923 as an optimization PBounds.add was not doing work when passed PBounds
was 
"empty".

Reported by allain.lalonde on 2010-01-14 20:04:54

mro commented 9 years ago

Reported by allain.lalonde on 2010-01-14 20:05:52

mro commented 9 years ago

Reported by heuermh on 2010-01-18 18:20:14

mro commented 9 years ago
This Issue will be resolved in 2.0. No binary incompatible changes are allowed until

2.0.

Reported by allain.lalonde on 2010-02-09 15:58:59

mro commented 9 years ago
Technically, this wouldn't be a binary-incompatible API change, since no APIs are
changed.  It is a behavior change though, and whether or not we should do that in a
.x release can be discussed.

Reported by heuermh on 2010-02-09 16:46:59

mro commented 9 years ago
No update on this issue in some time, would it be ok for me to apply this to release-1.3-branch
for Milestone-1.3.2?

Reported by heuermh on 2011-11-01 19:18:54

mro commented 9 years ago
See allain's comment in r970, at which he reverted r923.

Reported by atdixon on 2011-11-01 22:17:42

mro commented 9 years ago
See allain's comment in r970, at which he reverted r923. Does this mean that he had
realized some contract change that shouldn't go into a point release?

Reported by atdixon on 2011-11-01 22:18:37

mro commented 9 years ago
I wouldn't say that it is a contract change, since the PBounds javadoc doesn't make
any comment on this situation, other than to say that isEmpty() doesn't necessarily
mean that the bounds are empty.

Reported by heuermh on 2011-11-02 03:40:20

mro commented 9 years ago
I agree it doesn't sound like a contract change. His comment seemed to suggest some
clients were broken by the change:

"Reverting r923 since it was a breaking change. It will need to be redone in 2.0.
Seems that some apps were making use of this behaviour."

Reported by atdixon on 2011-11-04 04:17:17

mro commented 9 years ago

Reported by heuermh on 2013-11-26 21:11:15