[OpenJDK 2D-Dev] [9] RFR JDK-6842011: StackOverflowError printing landscape with scale and transform

[Jim Graham]

I think we need to go a bit further and change the way we describe them.  If we perhaps get very technical about how it 
is returning one element of the scaling equations/matrix then they will be discouraged from finding a simple use for it. 
  I'll try to come up with some wording today or tomorrow and it would be good to apply it to all 6 of the getters 
uniformly.  Something like:

Returns element M## of the transformation matrix which controls how the output XY coordinates are affected by the input 
XY coordinates.

Then on the getScaleXY methods add a "Note, this method will not return the amount by which input XY coordinates will be 
stretched or contracted since a 90 degree rotation will cause all of its contribution to be redirected into the other 
axis.  Properly determining the full scale of the matrix involves analyzing both this factor and the ...".

There is where it would be good to have the new methods ready to go so we can then immediately say ", such as in the 
getScalingFactorXY() method" or have an @see to send them where they need to go.  That doesn't mean we can't do this 
documentation refresh now, but we might want to make those new methods a high priority to get done soon.  (I'm 
guessing/hoping we can add small "fixup" APIs like that after FC since it doesn't really represent a "feature"...?)

			...jim

On 5/22/16 11:53 PM, prasanta sadhukhan wrote:
> Hi Jim,
>
>
> On 5/21/2016 3:20 AM, Jim Graham wrote:
>> We should acknowledge that the test case is buggy anyway.  It is not computing the scale of a transform correctly,
>> though that is likely due to the unfortunate naming we chose for our methods.
>>
>> If you are looking for "the amount by which an X coordinate is stretched or contracted", you have to compute a
>> distance formula on all of the elements of the X transform equation.  We don't have a method to do that for the
>> caller.  If we did, we might call it something very similar to "getScaleX()".
>>
>> Unfortunately, we have a method named "getScaleX()" which one might think does that, but it doesn't.
>>
>> While I think we should prevent a stack overflow here, it's really more of "making sure a program bug is caught early
>> and with a more sane response", than "fixing a valid test case".
>>
>> Also, we should consider adding a method to do the right calculation, and document the existing getScaleX() to point
>> out that it cannot be used to determine "the stretchiness of X coordinates" or something more appropriately worded...
>>
> I have documented the anomalies in getScaleX()/getScaley().
> http://cr.openjdk.java.net/~psadhukhan/6842011/webrev.01/
> I will create a bug to address this scaling calculation of a transform in affinetransform (as it is in geom package and
> not a printing issue par se). Will that be ok?
>
> Regards
> Prasanta
>>             ...jim
>>
>> On 5/20/16 4:27 AM, prasanta sadhukhan wrote:
>>> Hi Phil,
>>>
>>> When we call print() it calls RasterPrinterJob#printPage() which sets peekGraphics.transform([4.16,0,0][0,4.16,0]) as
>>> obtained from xscale=4.16 [getXRes()=300 / 72.0] ,yscale=4.16
>>> It calls SunGraphics2D.transform which was identity transform [1.0, 0.0, 0.0] [0.0, 1.0, 0.0] calls
>>> transform.concatenate(peekgraphicsTx) and stores as ([4.16,0,0][0,4.16,0])
>>>
>>> Then RasterPrinterJob#printPage() again calls peekGraphics.transform(new AffineTransform(page.getMatrix()));
>>> where page.getMatrix() returns 0.0, -1.0, 1.0, 0.0, 0.0, 841.88 and peekGraphics transform now becomes [0.0, 1.0, 0.0]
>>> [-1.0, 0.0, 841.88]
>>> which calls SunGraphics2D#transform() where it again does transform.concatenate(peekgraphicsTx)
>>>
>>> so the transform becomes [m00=0, m01=4.16, m02=0][m10=-4.16, m11=0, m12=3507.873]
>>> Now scaleX obtains value from g2d.getTransform().getScaleX() which returns SunGraphics2D stored transform.m00 which is 0
>>> and scaleY is m11=0 so scaleX,scaleY becomes 0.
>>>
>>> Regards
>>> Prasanta
>>> On 5/19/2016 4:03 AM, Phil Race wrote:
>>>> It sounds like scalex & scaley are 0 and are then used in calculations which
>>>> results in the NaN ? So why are they zero to begin with ?
>>>>
>>>> -phil.
>>>>
>>>> On 5/16/2016 3:32 AM, prasanta sadhukhan wrote:
>>>>> Hi All,
>>>>>
>>>>> Please review a fix for jdk9 whereby it is seen that
>>>>> A StackOverflowError occurs when printing in landscape orientation with a scaled and transformed graphics object.
>>>>>  at sun.print.PSPrinterJob.prepDrawing(PSPrinterJob.java:1610)
>>>>>     at sun.print.PSPrinterJob.beginPath(PSPrinterJob.java:1319)
>>>>>     at sun.print.PSPrinterJob.convertToPSPath(PSPrinterJob.java:1793)
>>>>>     at sun.print.PSPrinterJob$GState.emitPSClip(PSPrinterJob.java:1718)
>>>>>     at sun.print.PSPrinterJob.prepDrawing(PSPrinterJob.java:1625)
>>>>>
>>>>>  at sun.print.PSPrinterJob.beginPath(PSPrinterJob.java:1319)
>>>>>     at sun.print.PSPrinterJob.convertToPSPath(PSPrinterJob.java:1793)
>>>>>     at sun.print.PSPrinterJob$GState.emitPSClip(PSPrinterJob.java:1718)
>>>>>     at sun.print.PSPrinterJob.prepDrawing(PSPrinterJob.java:1625)
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-6842011
>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/6842011/webrev.00/
>>>>>
>>>>> StackOverflowError is occuring because the scalex, scaley for landscape orientation was 0 so when the testcase tries
>>>>> to scale with these scale factors
>>>>> using g2d.scale( 1 / scalex, 1 / scaley );
>>>>> it creates a AffineTransform of NaN transformation. Now, In linux, when the PS print drawing information is being
>>>>> prepared, it calls prepDrawing() where it checks
>>>>> getGState().mTransform.equals(mLastTransform) and since NaN values cannot be compared it results in "false", causing
>>>>> erroneous "grestore" postscript command to be issued and remove a GState from the stack so isOuterGState() becomes
>>>>> true which causes emitPSClip() to be called which calls
>>>>> prepDrawing() again via convertToPSPath() , beginPath() and since isOuterState() returns true due to transform not
>>>>> being equal it again calls emitPSClip() causing a recursion.
>>>>>
>>>>> The fix was to check if transform is NaN and do not fill the devicePath if it is so, so that erroeous drawing is not
>>>>> done.
>>>>> So, it will print out a blank page.
>>>>>
>>>>> In windows, the testcase prints out a blank page. In mac, the testcase prints a 2x2 rectangle.
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>
>>>>
>>>
>