Looks good. +1
...jim
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
