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

Phil Race philip.race at oracle.com
Tue May 31 17:42:24 UTC 2016 

On 05/26/2016 03:49 AM, prasanta sadhukhan wrote:
>
> Hi Phil,
>
>
> On 5/25/2016 10:10 PM, Philip Race wrote:
>> I am OK with committing the clarifying javadoc as part of this fix
>> and filing a new RFE for the new method - to be fixed at some later 
>> date.
>>
>> However before this fix can be committed you need to make sure
>> we have consistent behaviour across platforms and we do not have that 
>> yet
> In mac, unlike RasterPrinterJob which does the transform using the 
> following code
> /            pathGraphics.transform(scaleTransform);//
> //                // user (0,0) should be origin of page, not 
> imageable area//
> //pathGraphics.translate(-getPhysicalPrintableX(paper) / xScale,//
> //-getPhysicalPrintableY(paper) / yScale);//
> //                pathGraphics.transform(new 
> AffineTransform(page.getMatrix()));//
> /CPrinterJob does not do this transforms, it straightway goes to 
> native printloop code
>  so when scalex = g2d.getTransform().getScaleX(); is called the 
> transform returned is identity transform (as was the case in linux too 
> before the above transform code is executed)

I am not sure I know what you mean by that part in parentheses.
Our printing pipelines have always reflected the device transform.

If in fact OS X printing does not do that, then this is a bigger 
inconsistency ..
But not one we will try to address today, nor in JDK 9.

Verify that is the case and then file a P4 bug on that too in order to 
track it.

And this fix is then approved ..

-phil.


> so getScaleX/Y returns 1 as m00,m11=1 .
> So, since the transform is not invalid unlike linux, mac prints ok.
>
> Regards
> Prasanta
>>
>> -phil.
>>
>> On 5/23/16, 3:57 PM, Phil Race wrote:
>>> On 05/23/2016 03:33 PM, Jim Graham wrote:
>>>> 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?
>>>
>>> getScaleFactorX() should sort ahead of getScaleX()
>>>
>>> -phil.
>>>
>>>>
>>>> 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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160531/d79cee66/attachment.html>

More information about the 2d-dev mailing list