Re: [OpenJDK 2D-Dev] RFR 8144718: Pisces / Marlin Strokers may generate invalid curves with huge coordinates and round joins

2015-12-15 Thread Jim Graham
On second thought - it would be good to remove the dependency on the font from the test case since that can change over time and we'd no longer have the regression test we once had... ...jim On 12/7/15 5:30 PM, Jim Graham wrote: Looks good. ...jim On

Re: [OpenJDK 2D-Dev] RFR 8144718: Pisces / Marlin Strokers may generate invalid curves with huge coordinates and round joins

2015-12-15 Thread Jim Graham
That's a lot of test case for one path, but it does indeed fail before and pass after the fix so it looks like it does the job. +1 from me. ...jim On 12/15/15 2:06 PM, Laurent Bourgès wrote: Jim & Phil, Here is an updated webrev: http://cr.openjdk.java.net/~lbourges/m

Re: [OpenJDK 2D-Dev] RFR: 8145808: [PIT] test java/awt/Graphics2D/MTGraphicsAccessTest

2015-12-23 Thread Jim Graham
Looks good. You're correct that volatile isn't enough to protect the case where 2 threads finish their work at exactly the same time. They would need a synchronized block around the decrements (the increments are executed in the constructor by the main thread so they aren't contended, but it

Re: [OpenJDK 2D-Dev] RFR 8146076: Fail of sun/java2d/marlin/CeilAndFloorTests.java with Jigsaw

2016-01-08 Thread Jim Graham
For something this minor and essentially just bookkeeping in a test's comments, 1 review is fine. Phil is checking on your commit rights for the repo so you can (hopefully soon) push it directly... ...jim On 1/8/16 12:21 AM, Laurent Bourgès wrote: Hi, Another reviewe

Re: [OpenJDK 2D-Dev] Cleaner usage in java2 / awt

2016-01-08 Thread Jim Graham
We should probably be looking to replace Java2D's Disposer with the new Cleaner. The reason that we didn't use the prior non-public Cleaner was that it didn't support all reference types and we were using (I think) Phantom references? I think the new public Cleaner is more configurable so we

Re: [OpenJDK 2D-Dev] Review request for JDK-8147413 : api/java_awt/Image/MultiResolutionImage/index.html\#MultiResolutionRenderingHints[test_VALUE_RESOLUTION_VARIANT_BASE] started to fail

2016-01-21 Thread Jim Graham
BASE essentially just means to draw the 1:1 image. The problem is that returning null is not the way to achieve that. Returning null means "MRI doesn't apply here, just use the original image", but in the case of BASE we have to make sure it isn't an MRI first otherwise the caller will get an

Re: [OpenJDK 2D-Dev] [9]: RFR JDK-8147077, , IllegalArgumentException thrown by api/java_awt/Component/FlipBufferStrategy/indexTGF_General

2016-01-22 Thread Jim Graham
That looks good. While we are at it, do any of the other platforms or pipelines need the same fix applied? ...jim On 1/22/2016 4:01 AM, prasanta sadhukhan wrote: Hi All, Please review a JCK regression fix for JDK-8140530

Re: [OpenJDK 2D-Dev] [9]: RFR JDK-8147077, , IllegalArgumentException thrown by api/java_awt/Component/FlipBufferStrategy/indexTGF_General

2016-01-25 Thread Jim Graham
at id. Will that be ok? Regards Prasanta On 1/23/2016 4:34 AM, Jim Graham wrote: That looks good. While we are at it, do any of the other platforms or pipelines need the same fix applied? ...jim On 1/22/2016 4:01 AM, prasanta sadhukhan wrote: Hi All, Please review a JCK regressio

Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8148127, , IllegalArgumentException thrown by JCK test api/java_awt/Component/FlipBufferStrategy/indexTGF_General in opengl pipeline

2016-01-25 Thread Jim Graham
Looks just as good as the other fixes. Thanks for the additional testing and investigation! ...jim On 1/24/2016 10:37 PM, prasanta sadhukhan wrote: Hi All, Bug: https://bugs.openjdk.java.net/browse/JDK-8148127 webrev: http://cr.openjdk.java.net/~psadhukhan/8148127/web

Re: [OpenJDK 2D-Dev] [9] Review request for 8076545 Text size is twice bigger under Windows L&F on Win 8.1 with HiDPI display

2016-01-29 Thread Jim Graham
... ...jim On 1/29/2016 10:41 AM, Alexandr Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8076545/webrev.03 - LOGPIXELSX is changed to LOGPIXELSY On 11/16/2015 1:43 PM, Jim Graham wrote: Note that LOGPIXELSY is global and static between

Re: [OpenJDK 2D-Dev] Review request for 8069348 SunGraphics2D.copyArea() does not properly work for scaled graphics in D3D

2016-02-01 Thread Jim Graham
heck is removed from X11SurfaceData and GDIWindowSurfaceData Thanks, Alexandr. On 12/15/2015 11:22 AM, Jim Graham wrote: Also, X11SD and GDISD still reject TRANSLATESCALE. I think it may make sense for OSXOffscreen to return false for TRANSLATESCALE and punt back to the general implementation

Re: [OpenJDK 2D-Dev] Review request for JDK-8147413 : api/java_awt/Image/MultiResolutionImage/index.html\#MultiResolutionRenderingHints[test_VALUE_RESOLUTION_VARIANT_BASE] started to fail

2016-02-01 Thread Jim Graham
roval? Thanks, Jay -Original Message- From: Alexander Scherbatiy Sent: Friday, January 22, 2016 5:05 PM To: Jim Graham; Phil Race; Jayathirth D V Cc: 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] Review request for JDK-8147413 : api/java_awt/Image/MultiResolutionImage/index

Re: [OpenJDK 2D-Dev] [9] Review request for 8076545 Text size is twice bigger under Windows L&F on Win 8.1 with HiDPI display

2016-02-01 Thread Jim Graham
ics height rescaling Thanks, Alexandr. On 1/29/2016 1:16 PM, Jim Graham wrote: Hi Alexandr, Thanks for investigating the behaviors. With respect to using LOGPIXELSX or Y, these methods are used to scale both horizontal and vertical measurements so we really should have 2 scale values and resca

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-04 Thread Jim Graham
Hi Laurent, In AAShapePipe you load the values from abox[] into variables named xywh, but these are not xywh values, they are min/max values. We typically use any of the following naming conventions for these types of values: - x0, y0, x1, y1 - x1, y1, x2, y2 - minX, minY, maxX, maxY Other

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-04 Thread Jim Graham
r the TileState in case we have reentrance for the AAShapePipe code... ...jim On 2/4/2016 5:10 PM, Jim Graham wrote: Hi Laurent, In AAShapePipe you load the values from abox[] into variables named xywh, but these are not xywh values, they are min/max values. We typically use any o

Re: [OpenJDK 2D-Dev] [9] Review request for 8076545 Text size is twice bigger under Windows L&F on Win 8.1 with HiDPI display

2016-02-05 Thread Jim Graham
you review the updated fix: http://cr.openjdk.java.net/~alexsch/8076545/webrev.05 - The awt_DesktopProperties.cpp file is updated to use scaleX for width rescaling and scaleY for height rescaling Thanks, Alexandr. On 2/1/2016 5:51 PM, Jim Graham wrote: Hi Alexandr, In awt_DesktopPro

Re: [OpenJDK 2D-Dev] [9] Review request for 8142966 Wrong cursor position in text components on HiDPI display

2016-02-08 Thread Jim Graham
Isn't the problem there that we are returning an integer as the advance? Why aren't we returning 7.35 as a value instead of 8? Also, shouldn't 7.35 round to 7 and 14.7 round to 15? ...jim On 2/6/2016 7:28 AM, Alexander Scherbatiy wrote: On 05/02/16 23:39, Phil

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-08 Thread Jim Graham
o ensure rendering is correct PS: I noticed that AlphaPaintPipe has a constant int TILE_SIZE = 32 that seems useless as the method has a tilesize argument; maybe it should be cleaned up ? Regards, Laurent 2016-02-05 2:20 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com>>: Ah, at least one erro

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-08 Thread Jim Graham
stant int TILE_SIZE = 32 that seems useless as the method has a tilesize argument; maybe it should be cleaned up ? Regards, Laurent 2016-02-05 2:20 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com>>: Ah, at least one error spotted here after my initial re

Re: [OpenJDK 2D-Dev] [9] Review request for 8076545 Text size is twice bigger under Windows L&F on Win 8.1 with HiDPI display

2016-02-08 Thread Jim Graham
...jim On 2/6/16 7:19 AM, Alexander Scherbatiy wrote: On 2/5/2016 11:37 AM, Jim Graham wrote: Hi Alexandr, awt_DesktopProperties.cpp, line 300 - is the "1.0f /" a typo? Sorry. Here is the updated fix without the typo: http://cr.openjdk.java.net/~alexsch/

Re: [OpenJDK 2D-Dev] RFR 8149338: JVM Crash caused by Marlin renderer not handling NaN coordinates

2016-02-08 Thread Jim Graham
Is there a reason why you reversed the calculations for the slope at line 374? ...jim On 2/8/16 1:22 PM, Laurent Bourgès wrote: Phil & Jim, Please review a simple fix for a SEGV in Marlin renderer due to NaN coordinates: bug: https://bugs.openjdk.java.net/browse/JDK-81

Re: [OpenJDK 2D-Dev] RFR 8149338: JVM Crash caused by Marlin renderer not handling NaN coordinates

2016-02-08 Thread Jim Graham
In the test case, why are you using a log handler to check for a particular exception? Shouldn't any exception logged be cause for a test failure? ...jim On 2/8/16 1:22 PM, Laurent Bourgès wrote: Phil & Jim, Please review a simple fix for a SEGV in Marlin renderer du

Re: [OpenJDK 2D-Dev] RFR 8149338: JVM Crash caused by Marlin renderer not handling NaN coordinates

2016-02-09 Thread Jim Graham
On 2/9/16 12:51 AM, Laurent Bourgès wrote: + final double slope = (x1d - x2) / (y1d - y2); I prefer this syntax as it is more explicit that (x1d - x2) and (y1d - y2) are double values (not implicit promotion). I see. Arguably both are implicit promotion, though, no? Mathematiconceptually, "x

Re: [OpenJDK 2D-Dev] RFR 8149338: JVM Crash caused by Marlin renderer not handling NaN coordinates

2016-02-09 Thread Jim Graham
Let me know if/when you have an updated webrev. It should be good to go, but it couldn't hurt to publish one for the archives... ...jim On 2/9/16 12:51 AM, Laurent Bourgès wrote: Jim, Here are my answers to your 2 questions: 2016-02-09 0:14 GMT+01:00 Jim G

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-09 Thread Jim Graham
me but all tests are passing. Few anwsers below: 2016-02-08 23:33 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com>>: You might want to separate the new ReentrantTL class into a separate file in sun.java2d so it can be used in other places. What are the hurdles for using it in

Re: [OpenJDK 2D-Dev] RFR 8149338: JVM Crash caused by Marlin renderer not handling NaN coordinates

2016-02-10 Thread Jim Graham
eers, Laurent 2016-02-10 0:46 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com>>: Let me know if/when you have an updated webrev. It should be good to go, but it couldn't hurt to publish one for the archives... ...jim On 2/9/16 12:51 AM, Lauren

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-10 Thread Jim Graham
23:33 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com>>: You might want to separate the new ReentrantTL class into a separate file in sun.java2d so it can be used in other places. What are the hurdles for using it in Marlin instead of rolling its own? If it is going

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-10 Thread Jim Graham
:32 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com>>: One suggestion on ProviderTL - have a constructor overload that allows separate ref types for the TL and the CLQ references in case there is a situation where you want a hard reference for the primary context and a so

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-11 Thread Jim Graham
It all looks great. The comments are fine for internal documentation, but here is a suggestion for the initial comment on a couple of the Provider classes. I don't need to review the changes to comments. (NOTE: Watch out for trailing white space on these if you cut and past them back as the

Re: [OpenJDK 2D-Dev] Review Request for JDK-7116979 : Unexpected pixel colour when converting images to TYPE_BYTE_INDEXED

2016-02-11 Thread Jim Graham
Hi Jayathirth, Did you do any performance analysis of this change? You are adding 6 tests and multiple branches to per-pixel code. The effectiveness of this technique depends on the colormap that we have set up. For the BufferedImage.TYPE_INDEXED constructor we produce a fairly nice colorm

Re: [OpenJDK 2D-Dev] [9] Review request for 8076545 Text size is twice bigger under Windows L&F on Win 8.1 with HiDPI display

2016-02-15 Thread Jim Graham
herbatiy wrote: On 2/8/2016 3:04 PM, Jim Graham wrote: I don't understand the issue with the fonts that you are saying have different sizes for different DPIs. Those are pixel sizes, aren't they? They still need to be turned into user-space units for our applications to know what to do

Re: [OpenJDK 2D-Dev] Review Request for JDK-7116979 : Unexpected pixel colour when converting images to TYPE_BYTE_INDEXED

2016-02-15 Thread Jim Graham
541733 496217 1083466 959411 730527 728461 The changes are tested with plain images having primary colors like RED, GREEN, BLUE, BLACK and WHITE. Thanks, Jay -Original Message- From: Jim Graham Sent: Friday, February 12, 2016 4

Re: [OpenJDK 2D-Dev] JDK 9 RFR of JDK-8149896: Remove unnecessary values in FloatConsts and DoubleConsts

2016-02-16 Thread Jim Graham
Laurent may not technically be a reviewer, but he pretty much owns that code so if he's happy then I'm happy (but I didn't look at any of the other code outside of Marlin if you are keeping count of "full reviewers"...). ...jim On 2/16/2016 2:08 AM, Laurent Bourgès wro

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-18 Thread Jim Graham
webrev including jim's changes to comments: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8148886.5/ Hope it is ready to push, Regards, Laurent 2016-02-12 9:46 GMT+01:00 Laurent Bourgès mailto:bourges.laur...@gmail.com>>: Jim & Phil, 2016-02-11 23:20 GMT+01:00 Jim Gra

Re: [OpenJDK 2D-Dev] Review Request for JDK-7116979 : Unexpected pixel colour when converting images to TYPE_BYTE_INDEXED

2016-02-19 Thread Jim Graham
eded 120KB mailing list limit). Please search for "Final index value =" in the logs. Please let me know your inputs. Thanks, Jay -Original Message- From: Jim Graham Sent: Tuesday, February 16, 2016 1:03 AM To: Jayathirth D V Cc: 2d-dev@openjdk.java.net; Philip Race; Prasanta Sadh

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-19 Thread Jim Graham
a look to this updated webrev: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8148886.6/ I fixed the javadoc in ReentrantContextProvider: see below. 2016-02-18 23:40 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com>>: I swear I thought I remember asking this before and g

Re: [OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

2016-02-19 Thread Jim Graham
brev: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8148886.6/ I fixed the javadoc in ReentrantContextProvider: see below. 2016-02-18 23:40 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com> <mailto:james.gra...@oracle.com <mailto:james.gra...@oracle.com

Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel

2016-03-02 Thread Jim Graham
The logic here has mixed up the opacities and what needs to be done about them. If the source image is opaque, then this is not a BG operation at all because the bg color would not show through an opaque image, so checking the srcData is both wrong and should be a NOP here. If we get into tha

Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel

2016-03-03 Thread Jim Graham
updated webrev: http://cr.openjdk.java.net/~jdv/8139183/webrev.02/ Thanks, Jay -Original Message----- From: Jim Graham Sent: Thursday, March 03, 2016 5:19 AM To: Jayathirth D V; 2d-dev@openjdk.java.net; Philip Race; Prasanta Sadhukhan Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-813918

Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel

2016-03-07 Thread Jim Graham
ase find the updated webrev for review : http://cr.openjdk.java.net/~jdv/8139183/webrev.03/ Thanks, Jay -Original Message----- From: Jim Graham Sent: Friday, March 04, 2016 5:21 AM To: Jayathirth D V; Sergey Bylokhov Cc: 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] Review Req

Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel

2016-03-08 Thread Jim Graham
Hi Jay, On 3/8/16 12:56 AM, Jayathirth D V wrote: Hi Jim, Thanks for the review. I have made changes in test case based on input provided. 1) Added contentsLost() after we drawImage() and getSnapshot() to verify the content and if it is lost loop again. Unfortunately, if contents are lost yo

Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel

2016-03-09 Thread Jim Graham
10:44 PM, Jayathirth D V wrote: Hi Jim, I have made changes mentioned by you. Please find updated webrev: http://cr.openjdk.java.net/~jdv/8139183/webrev.05/ Thanks, Jay -Original Message- From: Jim Graham Sent: Wednesday, March 09, 2016 3:23 AM To: Jayathirth D V Cc: 2d-dev@openjdk.java.

Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel

2016-03-09 Thread Jim Graham
Also, line 59, "if{" => "if {"... ...jim On 3/9/16 3:30 PM, Jim Graham wrote: That looks good to go. A couple of code indentation issues in the test case: Per: http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.ht

Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel

2016-03-10 Thread Jim Graham
, Jayathirth D V wrote: Jim thanks for the review. Prasanta I have updated the webrev to address indentation issues. Please review: http://cr.openjdk.java.net/~jdv/8139183/webrev.06/ Thanks, Jay -Original Message- From: Jim Graham Sent: Thursday, March 10, 2016 5:28 AM To: Jayathirth D V Cc

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-10 Thread Jim Graham
Yes, those constructors should be type-safe. Perhaps that was done to save code in the caller, but that is a small price to pay to avoid errors in the future, and it makes sure there is a backup plan for different kinds of buffers... ...jim On 3/10/16 4:48 AM, Sergey

Re: [OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

2016-03-10 Thread Jim Graham
I can confirm that this is the same solution provided for the Ductus code so it will bring us to par with the former rasterizer. I would also check what happens when you try to render and createStrokedPath() for a path that is so full of NaN values that it never gets any segments out, and/or i

Re: [OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

2016-03-10 Thread Jim Graham
non-uniform scale of more than 2x, then it will still allow overflow in the final rendered path if a coordinate is near UPPER_BND... ...jim On 3/10/16 4:47 PM, Jim Graham wrote: I can confirm that this is the same solution provided for the Ductus code so it will bring us

Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel

2016-03-11 Thread Jim Graham
/ Thanks, Jay -Original Message- From: Jim Graham Sent: Friday, March 11, 2016 2:38 AM To: prasanta sadhukhan; Jayathirth D V Cc: 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-8139183 : drawImage misses background's alpha channel Yes, that is the only issue

Re: [OpenJDK 2D-Dev] [9] Review request for 8151303 [macosx] [hidpi] JButton's low-res. icon is visible when clicking on it

2016-03-11 Thread Jim Graham
Is this tied in with ImageFilter/FilteredImageSource? It looks like since FIS takes only a Producer, it has lost the handle to the image to get a source for a particular resolution. What happens if we introduce MultiResolution[Image]Producer? It would provide a single method that would mimic

Re: [OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

2016-03-11 Thread Jim Graham
Hi Laurent, On 3/11/16 9:06 AM, Laurent Bourgès wrote: Since this change will solve the issue for fills and uniform-scaled draws(), it handles the 90% case, but if you use a non-uniform scale of more than 2x, then it will still allow overflow in the final rendered path if a coordinate is near

Re: [OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

2016-03-11 Thread Jim Graham
Hi Laurent, On 3/11/16 9:06 AM, Laurent Bourgès wrote: By the way I started refactoring MRE.strokeTo () to get rid of outat ie I propose to remove the optimisation for non-uniform at as I prefer filtering transformed coordinates once even if it requires invDelta / Delta transform stages in the p

Re: [OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

2016-03-14 Thread Jim Graham
Hi Laurent, Did you consider adding a test with a completely degenerate path to make sure that we don't have any state errors if all of the segments are eaten by the "finite path" filter? I like your approach. It is worth noting that: - non-uniform scales are not common - your "always overf

Re: [OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

2016-03-14 Thread Jim Graham
Was this on a uniform scale? Did you do a test with a non-uniform scale? ...jim On 3/11/2016 3:10 PM, Laurent Bourgès wrote: Jim, Here are MapBench results comparing Marlin 0.7.3.2 (before) vs patched 0.7.3.3 (latest patch) on JDK 1.8_72: Synthetic results: Tests3612

Re: [OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

2016-03-15 Thread Jim Graham
veral corner cases... My comments below: 2016-03-15 0:34 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com>>: Hi Laurent, Did you consider adding a test with a completely degenerate path to make sure that we don't have any state errors if all of the segments are eate

Re: [OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

2016-03-18 Thread Jim Graham
reviewer, please. 2016-03-18 1:44 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com>>: Hi Laurent, That looks fine. If the subPathStarted changes are the only changes then it's good to go. If there was some other change that I missed, let me know so I can review it as

Re: [OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

2016-03-19 Thread Jim Graham
On 3/17/16 10:15 AM, Laurent Bourgès wrote: Hi Jim, Here is the updated webrev using your approach: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144938.4/ It is more close to 2nd webrev as it does not set subpathStarted to false in SEG_CLOSE case. Laurent 2016-03-17 1:49 GMT+01:00 Jim Gra

Re: [OpenJDK 2D-Dev] [2D-Dev] Review Request: JDK-8015070: Antialiased text on translucent backgrounds gets bright artifacts

2016-03-19 Thread Jim Graham
Hi Prahalad, This basically boils down to "alpha blending math needs to be performed in premultiplied form" and the existing code was not doing that. Your changes do add a pre-multiplication step to the math in two places - when you mix the src alpha and the glyph alpha at the top of the mac

Re: [OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

2016-03-19 Thread Jim Graham
in CrashNaNTest Comments below: 2016-03-15 23:45 GMT+01:00 Jim Graham mailto:james.gra...@oracle.com>>: The new tests look good. Thanks. There is one logic error in the new version of the degenerate coordinate filter. It is valid to have a PATH_CLOSE immediately followed by

Re: [OpenJDK 2D-Dev] CFV: New 2D Group Member: Alexander Scherbatiy

2016-03-21 Thread Jim Graham
Vote: yes ...jim On 3/17/16 3:49 PM, Philip Race wrote: I hereby nominate Alexander Scherbatiy to membership in the 2D group. Alexander Scherbatiy is a current member of the Swing group and has contributed almost 200 changesets to OpenJDK :- http://hg.openjdk.java.net/jdk9/dev/jdk/log?

Re: [OpenJDK 2D-Dev] [9] Review request for 8151303 [macosx] [hidpi] JButton's low-res. icon is visible when clicking on it

2016-03-21 Thread Jim Graham
We could do that for our own filters, but any random custom filter could run into the same issue, so it might not make sense to upgrade the existing filter mechanism to always attempt to do MR filtering. We could either create a parallel set of MR-aware filter mechanisms (such as the previousl

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-22 Thread Jim Graham
ava.net/~arapte/ajit/6353518/webrev.02/ Regards, Ajit -Original Message- From: Jim Graham Sent: Friday, March 11, 2016 2:40 AM To: Sergey Bylokhov; Ajit Ghaisas; 2d-dev Subject: Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer c

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-23 Thread Jim Graham
Point origin) { [not sure if that will make it through mail as intended]. Here in Raster.java, the first condition now seems to be redundant .. 890 if (dataType == DataBuffer.TYPE_BYTE && 891 dataBuffer instanceof DataBufferByte && -phil.

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-23 Thread Jim Graham
penjdk.java.net/~arapte/ajit/6353518/webrev.03/ Regards, Ajit -Original Message- From: Phil Race Sent: Wednesday, March 23, 2016 4:40 AM To: Jim Graham Cc: Ajit Ghaisas; Sergey Bylokhov; 2d-dev Subject: Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaste

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-23 Thread Jim Graham
...jim On 3/23/16 5:14 PM, Jim Graham wrote: For the record, in many places in the 2D code we also adopt a slight extension of the indentation rules such that the below might be written as: public ByteBandedRaster(SampleModel sampleModel,

Re: [OpenJDK 2D-Dev] [2D-Dev] Review Request: JDK-8015070: Antialiased text on translucent backgrounds gets bright artifacts

2016-03-23 Thread Jim Graham
for your time in review Have a good day Prahalad N. -Original Message- From: Jim Graham Sent: Friday, March 18, 2016 6:07 AM To: Prahalad Kumar Narayanan; 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [2D-Dev] Review Request: JDK-8015070: Antialiased text on translucent backgrounds ge

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-29 Thread Jim Graham
This sounded like such a great idea that I don't think we realized the one obvious reason we can't do this... You are talking about changing the method signatures on existing public constructors. While it might be nice to have had them typed from the get-go, I don't think we can change them n

Re: [OpenJDK 2D-Dev] [2D-Dev] Review Request: JDK-8015070: Antialiased text on translucent backgrounds gets bright artifacts

2016-03-29 Thread Jim Graham
brev link mentioned below and provide your suggestions. Webrev Link: http://cr.openjdk.java.net/~psadhukhan/prahlad/8015070/webrev.02/ Thank you for your time in review Have a good day Prahalad N. -Original Message- From: Jim Graham Sent: Thursday, March 24, 2016 7:57 AM To:

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-29 Thread Jim Graham
are in sun/awt which is fine. Apologies for the diversion... ...jim On 3/29/16 1:24 PM, Sergey Bylokhov wrote: On 29.03.16 23:19, Jim Graham wrote: This sounded like such a great idea that I don't think we realized the one obvious reason we can't do th

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-29 Thread Jim Graham
n this review already have indentation issues and fixing all of them will result in multiple changes masking the actual code changes that fixes the reported issue. Request you to review the updated webrev. Regards, Ajit -Original Message- From: Jim Graham Sent: Thursday, March 24,

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-29 Thread Jim Graham
switch test that all of the other blocks have... ...jim On 3/29/16 3:31 PM, Phil Race wrote: On 03/29/2016 02:37 PM, Jim Graham wrote: Raster.java, line 894: Why was the test for dataType removed from this if statement? In a previous version .. http://cr.openjdk.java.n

Re: [OpenJDK 2D-Dev] [2D-Dev] Review Request: JDK-8015070: Antialiased text on translucent backgrounds gets bright artifacts

2016-03-30 Thread Jim Graham
your suggestions. Thank you once again for detailed review and feedback Have a good day Prahalad N. -----Original Message- From: Jim Graham Sent: Wednesday, March 30, 2016 2:46 AM To: Prahalad Kumar Narayanan; 2d-dev@openjdk.java.net; Sergey Bylokhov Subject: Re: [OpenJDK 2D-Dev] [2

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-30 Thread Jim Graham
s apparent. -phil. On 3/29/16, 7:17 PM, Jim Graham wrote: One comes from the SampleModel, the other comes from the DataBuffer. The reason to check them both is to make sure they agree. There is no "it must be DataBuffer.TYPE_BYTE" here, there is only "it *should* be DataBuffer.TY

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-30 Thread Jim Graham
the DataBuffer. The webrev diff is insufficient to make this apparent. -phil. On 3/29/16, 7:17 PM, Jim Graham wrote: One comes from the SampleModel, the other comes from the DataBuffer. The reason to check them both is to make sure they agree. There is no "it must be DataBuffer.TYPE_BYTE"

Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

2016-03-30 Thread Jim Graham
Hi Ajit, On 3/30/2016 5:12 AM, Ajit Ghaisas wrote: Raster.java, line 986: Something to file as a potential bug for future work since the fix would have to be verified that it doesn't disrupt the other parts of this method, but... The set of if statements in this method never checked for a Ba

Re: [OpenJDK 2D-Dev] [9] Review request for 8073320 Windows HiDPI Graphics support

2016-03-31 Thread Jim Graham
Hi Alexandr, Is there a bug filed to upgrade JLightweightFrame to allow non-integer (and X/Y) scales? We'd need this capability to implement "Swing embedded in FX" correctly on Win8.1+... ...jim On 9/22/2015 2:33 AM, Alexander Scherbatiy wrote: Hello, Could you re

Re: [OpenJDK 2D-Dev] Review Request for JDK-8153363: Redundant check for number of components in PackedColorModel.equals() method

2016-04-04 Thread Jim Graham
Looks good... ...jim On 4/4/16 8:35 AM, Jayathirth D V wrote: Hi, _Please review the following fix in JDK9:_ __ Bug : https://bugs.openjdk.java.net/browse/JDK-8153363 Webrev : http://cr.openjdk.java.net/~jdv/8153363/webrev.00/ Issue : We have redundant check for equ

Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies on ColorModel.equals() which is not strict

2016-04-06 Thread Jim Graham
Are there any bugs in the database about the fact that many of these ColorModel subclasses implement equals without hashcode? They really should both be implemented, but since ColorModel implements the method based on its tests, they are technically covered by the equals/hashCode contract - it

Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies on ColorModel.equals() which is not strict

2016-04-07 Thread Jim Graham
Hi Jayathirth, This looks good. One thing to note for efficiency, "instanceof" also checks for null. It only returns true for non-null objects, so you don't need to explicitly test for null at the top of ICM.equals(). (Though, you should include a test(s) in the unit test that verifies that

Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies on ColorModel.equals() which is not strict

2016-04-11 Thread Jim Graham
i Jim, Thanks for the review. I have made all recommended changes and created new subtask for JDK-6588409(https://bugs.openjdk.java.net/browse/JDK-8153943 ). Please review updated webrev: http://cr.openjdk.java.net/~jdv/7107905/webrev.02/ Thanks, Jay -Original Message- From: Jim Graham

Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies on ColorModel.equals() which is not strict

2016-04-12 Thread Jim Graham
/7107905/webrev.03/ Thanks, Jay -Original Message- From: Jim Graham Sent: Tuesday, April 12, 2016 12:21 AM To: Jayathirth D V; Philip Race; Prasanta Sadhukhan Cc: 2d-dev@openjdk.java.net Subject: Re: Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies

Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies on ColorModel.equals() which is not strict

2016-04-12 Thread Jim Graham
On 4/12/2016 12:59 PM, Phil Race wrote: I am catching up on email here, and "+1" but a couple of comments - I suppose that we can't learn anything useful from "cm.validbits.equals(this.validbits)" since only the bits up to "map_size" should be tested ? Perhaps if the constructors truncated i

Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies on ColorModel.equals() which is not strict

2016-04-13 Thread Jim Graham
ions. Please find updated webrev for review : http://cr.openjdk.java.net/~jdv/7107905/webrev.04/ Thanks, Jay -Original Message- From: Phil Race Sent: Wednesday, April 13, 2016 1:49 AM To: Jim Graham Cc: Jayathirth D V; Prasanta Sadhukhan; 2d-dev@openjdk.java.net Subject: Re: Review Request

Re: [OpenJDK 2D-Dev] Review Request for JDK-7116979 : Unexpected pixel colour when converting images to TYPE_BYTE_INDEXED

2016-04-22 Thread Jim Graham
start and then gray values. So we are picking final gray value and not in between white value. That's why I am not verifying white color validity in test case. Please find updated webrev for review : http://cr.openjdk.java.net/~jdv/7116979/webrev.01/ Thanks, Jay -Original Message-

Re: [OpenJDK 2D-Dev] Review Request for JDK-8153943 : In java.awt.image package some of the classes are missing hashCode() or equals() method

2016-04-22 Thread Jim Graham
This is actually a pretty nasty issue that Joe Darcy also brought up in the CCC review. In order to have symmetric testing of .equals(), we pretty much have to enforce this test at all levels, including in the original ColorModels.equals() method. If we don't enforce this in CM.equals(), the

Re: [OpenJDK 2D-Dev] [2D-Dev] Review Request: JDK-8015070: Antialiased text on translucent backgrounds gets bright artifacts

2016-04-26 Thread Jim Graham
; \ } \ } while (0); My apologies if the above code did not appear on the final webrev email. ( In few instances, the newlines don't appear in plain-text format ) Kindly review the changes present in webrev and provide your views. If the changes are good, we could take up for the cod

Re: [OpenJDK 2D-Dev] [9] Review request for 8152309 Seamless way of using image filters with multi-resolution images

2016-04-26 Thread Jim Graham
This all snowballed pretty far and wide. The original fix to Swing icons was trivial in comparison. In retrospect it might be better to simply offer the helper method from the original bug fix as a Toolkit solution and force applications to adopt it when dealing with MR sources: http://cr.op

Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies on ColorModel.equals() which is not strict

2016-04-26 Thread Jim Graham
review in its thread. Thanks, Jay -----Original Message- From: Jim Graham Sent: Thursday, April 14, 2016 2:57 AM To: Jayathirth D V; Philip Race Cc: 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies on Color

Re: [OpenJDK 2D-Dev] Review Request for JDK-8153943 : In java.awt.image package some of the classes are missing hashCode() or equals() method

2016-04-26 Thread Jim Graham
On 4/25/16 8:48 AM, Sergey Bylokhov wrote: On 23.04.16 4:59, Jim Graham wrote: So, I'd recommend that CM.equals() tests getClass() == getClass() at the base level and then all others will use super.equals() and get the same protection. It means you can't have a subclass of CCM be &

Re: [OpenJDK 2D-Dev] Review Request for JDK-8153943 : In java.awt.image package some of the classes are missing hashCode() or equals() method

2016-04-26 Thread Jim Graham
, Jay -Original Message- From: Jim Graham Sent: Saturday, April 23, 2016 7:30 AM To: Phil Race; Jayathirth D V Cc: 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-8153943 : In java.awt.image package some of the classes are missing hashCode() or equals() method

Re: [OpenJDK 2D-Dev] [2D-Dev] Review Request: JDK-8015070: Antialiased text on translucent backgrounds gets bright artifacts

2016-04-27 Thread Jim Graham
our views. Thank you once again for your effort in review Have a great day Prahalad N. -Original Message- From: Jim Graham Sent: Wednesday, April 27, 2016 2:12 AM To: Prahalad Kumar Narayanan; 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [2D-Dev] Review Request: JDK-8015070: Antiali

Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: equals() method in IndexColorModel doesnt exist and it relies on ColorModel.equals() which is not strict

2016-04-29 Thread Jim Graham
107905/webrev.06/ I will make changes in CCC after technical review is approved. Thanks, Jay -Original Message----- From: Jim Graham Sent: Wednesday, April 27, 2016 4:24 AM To: Jayathirth D V; Philip Race Cc: 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905:

Re: [OpenJDK 2D-Dev] Review Request for JDK-7116979 : Unexpected pixel colour when converting images to TYPE_BYTE_INDEXED

2016-04-29 Thread Jim Graham
ease find updated webrev for review : http://cr.openjdk.java.net/~jdv/7116979/webrev.02/ Thanks, Jay -Original Message- From: Phil Race Sent: Tuesday, April 26, 2016 11:56 PM To: Jim Graham Cc: Jayathirth D V; 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-711

Re: [OpenJDK 2D-Dev] Review Request for JDK-7116979 : Unexpected pixel colour when converting images to TYPE_BYTE_INDEXED

2016-05-01 Thread Jim Graham
can vary if user provides custom color-map. This looks like completely different issue of not extracting proper value for white color. Thanks, Jay -Original Message- From: Jim Graham Sent: Saturday, April 30, 2016 4:43 AM To: Jayathirth D V; Philip Race Cc: 2d-dev@openjdk.java.net Subject: Re:

Re: [OpenJDK 2D-Dev] [2D-Dev] Review Request: JDK-8015070: Antialiased text on translucent backgrounds gets bright artifacts

2016-05-02 Thread Jim Graham
ng used. 4. As with every change in the native code, Internal automated build system was used to ensure successful compilation across all platforms. Kindly review the changes and provide your opinion. Thank you Have a good day Prahalad N. -Original Message----- From: Jim Graham Sent: Thur

Re: [OpenJDK 2D-Dev] Review Request for JDK-8153943 : In java.awt.image package some of the classes are missing hashCode() or equals() method

2016-05-03 Thread Jim Graham
... ...jim On 5/3/2016 2:00 PM, Phil Race wrote: On 04/26/2016 04:10 PM, Jim Graham wrote: The ComponentColorModel implementation is a meaningless wrapper around super.equals/hashCode(). Why does it not test CCM-specific fields? It should although this looks like it is more than just running through

Re: [OpenJDK 2D-Dev] Review Request for JDK-8153943 : In java.awt.image package some of the classes are missing hashCode() or equals() method

2016-05-05 Thread Jim Graham
on in java.awt.image.ComponentSampleModel and its subclassses". Please provide your inputs. Thanks, Jay -Original Message----- From: Jim Graham Sent: Wednesday, May 04, 2016 2:44 AM To: Phil Race Cc: 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-81

Re: [OpenJDK 2D-Dev] [9] Review Request: Cleanup of sun.java2d.pipe.Region

2016-05-12 Thread Jim Graham
This looks great! I guess the final was removed from the getters because the class is now final, though it probably doesn't hurt anything to leave them final. In any case, you only removed it from 3 of them, so it is unbalanced now. I suppose Hotspot is smart enough to inline accessors in a

Re: [OpenJDK 2D-Dev] [9] Review Request: Cleanup of sun.java2d.pipe.Region

2016-05-12 Thread Jim Graham
Looks good +1. Is there a bug filed for this? ...jim On 5/12/16 1:50 PM, Sergey Bylokhov wrote: This looks great! I guess the final was removed from the getters because the class is now final, though it probably doesn't hurt anything to leave them final. In any case,

Re: [OpenJDK 2D-Dev] [9] Review request for 8151303 [macosx] [hidpi] JButton's low-res. icon is visible when clicking on it

2016-05-13 Thread Jim Graham
ion variants of one multi-resolution image to another: Image image = // original image Image filteredImage = MultiResolutionImage.map(image, (img) -> /* return filtered image */); Thanks, Alexandr. On 21/03/16 22:31, Jim Graham wrote: We could do that for our own fi

Re: [OpenJDK 2D-Dev] [9] Review request for 8151303 [macosx] [hidpi] JButton's low-res. icon is visible when clicking on it

2016-05-13 Thread Jim Graham
Another reason to avoid new API is that we don't have to involve the CCC to get this "bug fix" in... ...jim On 5/13/16 3:50 PM, Jim Graham wrote: That looks very tight. The only issue I'd have is that it would be better if this could be done wit

<    2   3   4   5   6   7   8   9   >