[9] Review request: 8180337: JavaFX 9 msg drop 40 l10n resource file update

2017-05-15 Thread Kevin Rushforth

I am posting this review on behalf of Leo Jiang (ljiang).

https://bugs.openjdk.java.net/browse/JDK-8180337
http://cr.openjdk.java.net/~kcr/8180337/webrev/

This is the last planned translation drop for localized resource message 
for FX in JDK 9.


-- Kevin



[8] Code Review Request For 8180180: [Mac] WebView renders icons instead of letters on some sites

2017-05-15 Thread Dipak Kumar
Hi Philip, Kevin and Arun,

 

Please review the below changes:

 

JBS: https://bugs.openjdk.java.net/browse/JDK-8180180 

Webrev: http://cr.openjdk.java.net/~arajkumar/dipak/8180180/webrev/ 

 

Please note that this is a backport for JBS - HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8088205"JDK-8088205 . 

 

Thanks,

Dipak

 


Re: MarlinFX upgrade 0.7.5

2017-05-15 Thread Jim Graham

Hi Laurent,

I didn't notice anything functionally wrong with reviewing the webrev.  I'm going to do some basic testing on it now 
while you create a JBS bug for it and submit for a formal review.  But I did notice some discrepancies by diffing the 
sources against each other which it would be nice to know if they were oversights or "future work".  If there is 
something you think should be addressed now, do that in the process of switching to a formal review (with JBS #) on it...


I did a bit of diffing:

- Marlin2D against MarlinFX
- *NoAA against the AA versions

and noticed:

- RendererNoAA and DRendererNoAA line 38 are missing a "d" suffix.
- RendererNoAA and DRendererNoAA lines 61,90 - missing comment about test
- Helpers and DHelpers - some adjustment of "pts[ off+1 ]" lines that didn't 
happen in 2D
- Configurable constants for Inc/Dec/Quad/Cubic in 2D, but not FX
- DRendererSharedMemory in FX, but not 2D
- FloatMath.ceil_f deleted from FX, but not 2D (not used in 2D)
- (FloatMath.floor_f is also only in 2D, but it is used in 
MarlinRenderingEngine)

...jim

On 5/11/17 2:14 PM, Laurent Bourgès wrote:

Hi Jim,

Please review this updated webrev:
http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-075.1/

I mainly synchronized again all my Marlin repositories and this provides an
up-to-date patch in synch with Marlin2D.

Changes:
- Renderer (4 variants): use shared memory among AA and NonAA renderer
- Fixed comments in Helpers, XXXArrayCache
- Marlin2D backports in ArrayCacheConst, Renderer ...
- SWUtils: remove import + JavaShapeRenderer made static
- fixed copyright year to 2017

You can see the incremental changes by comparing the openjfx10.patch:
diff marlinFX-075.0/openjfx10.patch marlinFX-075.1/openjfx10.patch

Regards,
Laurent


2017-04-26 7:06 GMT+02:00 Jim Graham :


I've reviewed the code and run a number of tests.  Things look fine.

I spotted at least one thing that I brought up in the 2D Marlin review,
but since the 2 source bases are moving towards synchronizing with each
other I didn't look too closely since many of the changes in the 2D Marlin
update are things that are already "fixed" in this FX Marlin code, so I
thought I would focus my scrutiny more on the 2D review instead. Would this
code base be affected by the review comments I made there?  Did you want to
hold both until they both are ready to go in and then push them at the same
time (to keep them in sync)?

Minimally, it is time to file a bug against FX for this...

...jim


On 4/19/17 11:35 PM, Laurent Bourgès wrote:


Hi,

Please review this MarlinFX upgrade to Marlin 0.7.5:

JBS: no bug yet for OpenJFX 10
webrev: http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-075.0/

Changes:
- Renderers: fixed block processing
- dead code & few comment removals in Strokers
- fixed all floating-point number literals to be x.0f or x.0d to simplify
the conversion between float & double variants

PS: I plan to run later FindBugs, Netbeans & IntelliJ code analysis tools
to fix any warning

Cheers,
Laurent