[OpenJDK 2D-Dev] [9] RFR JDK-6842011: StackOverflowError printing landscape with scale and transform
Philip Race
philip.race at oracle.com
Wed Jun 1 16:32:23 UTC 2016
+1
-phil.
On 6/1/16, 12:03 AM, prasanta sadhukhan wrote:
>
> HI Phil, Jim,
>
> I have modified webrev to address Jim's doc change. Please review
> http://cr.openjdk.java.net/~psadhukhan/6842011/webrev.02/
>
> I will send the ccc link separately to you for approval.
>
> On 5/31/2016 11:12 PM, Phil Race wrote:
>> 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.
> What I meant was SunGraphics2D.transform was an identity transform in
> linux and osx to begin with, but in linux RasterPrinterJob calls the
> above code snippet which modifies SunGraphics2D.transfrom from
> identity transform to
> [m00=0, m01=4.16, m02=0][m10=-4.16, m11=0, m12=3507.873] but it
> remains as identity transform in osx.
>> 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.
>>
> RasterPrinterJob#print() calls printPage() which does this device
> transformation but in osx CPrinterJob#print calls printloop() in osx
> native which does not do this. I have created JDK-8158339 to address this.
>
> Regards
> Prasanta
>> 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/20160601/96322612/attachment.html>
More information about the 2d-dev
mailing list