samreid / piccolo2d

Automatically exported from code.google.com/p/piccolo2d
0 stars 0 forks source link

toImage doesn't return an image of the proper size #88

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?  The following test fails:
    public void testToImageResultsInDesiredSizeImage() {
    PNode node = new PNode();
    node.setBounds(0, 0, 10, 10);       

    BufferedImage img = (BufferedImage)node.toImage(20, 40, null);
    assertEquals(40, img.getHeight(null));
    assertEquals(20, img.getWidth(null));   
    }

What is the expected output? What do you see instead?
  I'd expect to get an image with the dimensions I provided back and the
node stretched to cover as much of it as it could.

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

Please provide any additional information below.
  It's seems that the logic within the function is correctly attempting to
resize the node to be as large as it can be, it's just that it's not
returning an image of the dimensions provided.  I'll submit a patch once I
know this is not the intended behaviour.

Original issue reported on code.google.com by allain.lalonde on 11 Jul 2009 at 11:42

GoogleCodeExporter commented 9 years ago
Hmm, this is one of those cases where having final method parameters

    public Image toImage(final int width, final int height, final Paint backGroundPaint)

instead of

    public Image toImage(int width, int height, Paint backGroundPaint)

may have helped prevent the problem.

Either width or height is rescaled, so if the node's fullBounds is not square 
you may
not get what you asked for.  I think all that's needed here is some 
clarification to
the javadoc, and test cases that cover the different conditions (i.e. narrow 
full
bounds, square full bounds, wide full bounds).

Original comment by heue...@gmail.com on 13 Jul 2009 at 3:08

GoogleCodeExporter commented 9 years ago
Turns out that the resizing is handled automatically in the PNode.print method, 
so the 
code in toImage doing the resizing was unnecessary, and actually caused toImage 
to 
behave in a way that I think is strange.  If I ask for an image of dimension 
w,h I 
should get one of that size back. And the node should be drawn to fill that 
shape as 
much as possible.  That is its current behavior (post fix).

Original comment by allain.lalonde on 13 Jul 2009 at 7:26

GoogleCodeExporter commented 9 years ago
Just a remark: There are various sensible ways to expand one rectangle to fit 
into
another.

1) it can be just strechted in both directions
2) "AspectFit" - it can be streched to fit into but preserve aspect ratio
2) "AspectFill" - it can be streched to completely fill but preserve aspect 
ratio

Should be an input parameter.

Original comment by mr0...@mro.name on 23 Jul 2009 at 11:12

GoogleCodeExporter commented 9 years ago
Good idea. I'd probably write it to have the default be #1 and have another
overloaded method that accepts a parameter, since #1 it retains binary 
compatability
and #2 I think stretching it is a sensible default.

Agreed?

Original comment by allain.lalonde on 24 Jul 2009 at 12:14

GoogleCodeExporter commented 9 years ago
Moving this one back to status Started, since work is still ongoing.

Original comment by heue...@gmail.com on 28 Jul 2009 at 1:58

GoogleCodeExporter commented 9 years ago
Fixed in r649

Original comment by allain.lalonde on 5 Aug 2009 at 4:06

GoogleCodeExporter commented 9 years ago
I was not able to validate this fix.  I've added an example that demonstrates 
the
different fill strategies, and all the aspect cover images have no foreground.  
The
aspect fill and exact fill images look correct to me.

$ svn commit -m "Issue 88 ; adding example that demonstrates the different 
toImage
fill strategies, uncovered a possible bug with aspect cover fill strategy"
Adding         
examples/src/main/java/edu/umd/cs/piccolo/examples/ToImageExample.java
Transmitting file data ...
Committed revision 784.

Original comment by heue...@gmail.com on 21 Oct 2009 at 3:39

GoogleCodeExporter commented 9 years ago
Attaching some screen shots.  On 32-bit linux with open-jdk 1.6, the middle 
exact
fill image also has no foreground.

Moving this issue and Issue 106 back to Started status.

Original comment by heue...@gmail.com on 21 Oct 2009 at 8:00

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Since the unit tests pass now, this is fixed.

Original comment by allain.lalonde on 21 Oct 2009 at 9:09

GoogleCodeExporter commented 9 years ago
Works for me now on Windows, middle aspect cover and middle exact fill images 
still
have no foreground on open-jdk 1.6.  New screenshot attached.

Original comment by heue...@gmail.com on 22 Oct 2009 at 3:27

Attachments:

GoogleCodeExporter commented 9 years ago
I was able to reproduce the bug with OpenJDK on my Ubuntu box, but I think it's
unrelated to the toImage code. To test this out yourself, just set the scale of 
the
aspectCover PText to 10 and run the toImage Example. The label shouldn't even 
appear
on screen. It seems that when the font gets too big, OpenJDK doesn't bother 
rendering
them. Yippee! Groan.

Original comment by allain.lalonde on 22 Oct 2009 at 4:19

GoogleCodeExporter commented 9 years ago
Added texture-based examples to ToImageExample and confirm that the problems 
seen
above with open-jdk 1.6 are due to its own font rendering problems.  Marking as 
Verified.

$ svn commit -m "Issues 88, 106 ; adding texture-based examples to 
ToImageExample"
...
Committed revision 871.

Original comment by heue...@gmail.com on 30 Oct 2009 at 3:51