[OpenJDK 2D-Dev] [9] RFR JDK-6842011: StackOverflowError printing landscape with scale and transform
prasanta sadhukhan
prasanta.sadhukhan at oracle.com
Wed Jun 1 07:03:22 UTC 2016
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/2b824911/attachment.html>
More information about the 2d-dev
mailing list