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

Philip Race philip.race at oracle.com
Wed May 25 16:40:09 UTC 2016 

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

-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
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>
>

More information about the 2d-dev mailing list