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

[Jim Graham]

Though, they are likely to think this API is doing that.  We have a 
visibility problem here to make sure that any work going forward is more 
likely to see the new method and ignore this one.  I don't think we'll 
win there on naming alone, but we can make the javadocs look very 
intimidating so if they are using completion they may get scared and 
hopefully see the other method before they just accept the completion. 
Perhaps we can try to make the alphabetic sorting have the new methods 
appear first in the list?

In FX we were smart and went with very dry "getMxy()" style names that 
won't attract attention...

			...jim

On 5/23/2016 3:16 PM, Phil Race wrote:
> What we have here might happen when developer A writes some UI code
> without any conception that a 90 degree rotation may be in effect and
> then developer B
> comes along and adds printing support .. and the implementation rotates it.
> So an out-of-the-box advertised API that does what dev A really meant
> would be helpful.
>
> -phil.
>
> On 05/23/2016 11:52 AM, Jim Graham wrote:
>> 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
>>>>>>
>>>>>>
>>>>>
>>>
>