[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] Review Request: JDK-6457721: Stroke width incorrect while printing
From:       Prahalad Kumar Narayanan <prahalad.kumar.narayanan () oracle ! com>
Date:       2016-06-22 6:47:00
Message-ID: acd905b5-d8a6-4ceb-8f02-02c2b0f8ed6b () default
[Download RAW message or body]

Hello Phil, Jim and Prasanta

Thanks for each of your time in review feedback.
I will continue my inspection and revert once I 've a concrete solution.

Thank you
Have a good day

Prahalad N.

-----Original Message-----
From: Philip Race 
Sent: Wednesday, June 22, 2016 5:02 AM
To: Jim Graham
Cc: prasanta sadhukhan; Prahalad Kumar Narayanan; 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] Review Request: JDK-6457721: Stroke width incorrect \
while printing

For a different situation, PathGraphics does this :-

         if ((getTransform().getType()
& ~( AffineTransform.TYPE_UNIFORM_SCALE
                   | AffineTransform.TYPE_TRANSLATION
                   | AffineTransform.TYPE_QUADRANT_ROTATION
                   )) != 0) {
             return false;
         }

The methods here could all be updated to apply a similar check and then I think you'd \
be OK for these cases - but some testing is needed to make sure GDI co-operates. Hmm \
.. I applied it and it helped some cases but not all when I applied a non-uniform \
scale. Either I did something wrong or the code we are delegating (upwards) to has a \
problem as well.


The test needs to be expanded to handle all the cases and also the custom stroke \
path.

Also it would be very helpful if the test displayed to the screen as well.
One thing you can do to test the printed output is -Dsun.java2d.print.pipeline=raster

This makes it use "on screen" loops to generate the print out as one big image. It \
can be a quick way to verify the printing pipelines are handling it right.

-phil.

On 06/07/2016 02:50 PM, Jim Graham wrote:
> The calculations are still not correct.  Consider a transform that 
> zooms Y coordinates and shrinks X coordinates, you will think that the 
> resulting line width is large, and it will be for horizontal lines.
> But vertical lines will experience a smaller line width.
> 
> The only real calculation to see if the point size needs to be 
> adjusted is one of:
> 
> - if the direction is known, use an N-length vector perpendicular to 
> the direction of the path.  This might work for drawLine(), but not 
> for drawPolygon() and draw(Shape).
> 
> - if the direction is not known, or varies as in a polygon or path, 
> then you need to use computations to compute the size of the minor 
> axis of the ellipse produced by transforming the unit circle, which is 
> not an easy problem (you can look at the code in the Marlin renderer 
> to see how it handles this, I don't recall how it does it off-hand).
> 
> Also, you fixed the computations of "is the line width too thin", but 
> there are also computations for "if it is too thin, then what is the 
> correct minimum line width?" that still use the old computations, but 
> in reverse...
> 
> ...jim
> 
> On 6/7/2016 4:32 AM, prasanta sadhukhan wrote:
> > Fix looks good to me. But, do we not need to do the same for
> > 
> > 209                     minLineWidth = Math.max(Math.abs(minPenSize.x)
> > 210                                             
> > Math.abs(minPenSize.y));
> > 
> > For the test the instruction textarea is editable which needs to be 
> > made read-only.
> > Also, if the user does not do any manual intervention, the test passes.
> > The test should have provision of failing if there is no user 
> > interaction. I guess there is no need of using Robot class in a 
> > manual test, you can use sleep if it is just meant to delay main() closing.
> > The test has some wildcard imports which can be changed to specific 
> > imports.
> > 
> > Regards
> > Prasanta
> > On 6/7/2016 3:42 PM, Prahalad Kumar Narayanan wrote:
> > > 
> > > Hello Everyone on Java2D
> > > 
> > > 
> > > 
> > > Good day to you.
> > > 
> > > 
> > > 
> > > Please find herewith: Webrev changes for the bug
> > > 
> > > Bug ID     : JDK-6457721
> > > 
> > > Bug Link : https://bugs.openjdk.java.net/browse/JDK-6457721
> > > 
> > > 
> > > 
> > > As per the Bug:
> > > 
> > > 1.       With rotation angle set to Graphics2D object, the printed
> > > Lines, Rectangles or Shapes have incorrect stroke width
> > > 
> > > 2.       The behavior is typically noticed at a rotation of 45'
> > > 
> > > 3.       The issue is reproducible on JDK 9 and is applicable for
> > > windows platform.
> > > 
> > > 
> > > 
> > > Quick Description on Changes
> > > 
> > > 1.       Thanks to Phil for adding his detailed comments on JBS. They
> > > were really helpful.
> > > 
> > > a.       Root Cause: The formula for stroke width calculation was
> > > incorrect in the WPathGraphics.java code.
> > > 
> > > 
> > > 
> > > 2.       Following changes have been incorporated-
> > > 
> > > a.       Correction to the stroke width calculation.
> > > 
> > > i.
> > > As Phil suggested in his JBS comments, the logic has been modified 
> > > to use distance formula after being rotated by theta.
> > > 
> > > ii.
> > > Since many other places also use the same logic, correction has been 
> > > taken up wherever necessary
> > > 
> > > 
> > > 
> > > b.      Fallback path for Lines and Rectangles when Custom stroke
> > > implementation is set.
> > > 
> > > i.
> > > It was observed that when lines or rectangles are drawn with custom 
> > > stroke object, the code does not print any of these shapes.
> > > 
> > > ii.
> > > A fallback logic is now provided that prints the lines and 
> > > rectangles as 'Shapes'.
> > > 
> > > iii.
> > > The same fallback logic is being taken up in other sections of code, 
> > > when GDI fails to instantiate a pen with required Attributes.
> > > 
> > > Hence the introduced fallback will be consistent with 
> > > existing code.
> > > 
> > > 
> > > 
> > > c.       Manual Test Case to detect the bug and verify the fix
> > > 
> > > i.
> > > A manual jtreg test case has been provided for users to run, print 
> > > and verify the fix.
> > > 
> > > 
> > > 
> > > Detailed comments have been added for changes in the code.
> > > 
> > > Kindly review the changes at your convenience and provide your 
> > > suggestions.
> > > 
> > > Webrev Link:
> > > <http://cr.openjdk.java.net/%7Epnarayanan/6457721/webrev.00/>http://
> > > cr.openjdk.java.net/~pnarayanan/6457721/webrev.00/
> > > 
> > > 
> > > 
> > > 
> > > Thank you for your time in review
> > > 
> > > Have a good day
> > > 
> > > 
> > > 
> > > Prahalad N.
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > 


[prev in list] [next in list] [prev in thread] [next in thread]