Re: [OpenJDK 2D-Dev] RFR: 8035302: Eliminate dependency on sun.nio.cs from AWT and Motif code
Updated webrev http://cr.openjdk.java.net/~prr/8035302.2/
- removed the jdk.charsets export to java.desktop from modules.xml
- removed the COMPOUND_TEXT support which likely has not been used since
Motif support was removed
-phil.
On 3/20/15 2:39 PM, Phil Race wrote:
On 3/19/15 8:19 PM, Mandy Chung wrote:
On 03/16/2015 02:32 PM, Phil Race wrote:
Here is an updated fix that instead of removing the sun.nio
dependency instead removes the jdk.charsets static dependency. From
the internal API point of view its sun.nio.cs.ext that is the
issue, not sun.nio
http://cr.openjdk.java.net/~prr/8035302.1/
Looks quite good and much cleaner.
src/jdk.charsets/unix/classes/sun/nio/cs/ext/CompoundTextSupport.java
cls = Class.forName("sun.awt.motif." + enc);
Should we do something about this Class.forName?
Hmm .. that class is littered with references to X11 !
eg
if (isEncodingSupported("X11GB2312")) {
Sherman : why is all of this here ?
modules.xml - you can take out jdk.charsets
from java.desktop module. You can verify the module access
by make verify-modules target.
There are tests referencing sun.awt.motif that I assume you will
update and send out another webrev?
Tests under test/java/awt are expected and surprisingly there are
tests in test/sun/nio/cs:
test/sun/nio/cs/TestX11CNS.java
test/sun/nio/cs/TestX11JIS0201.java
I am getting rid of these
thank you for doing this.
Mandy
Re: [OpenJDK 2D-Dev] [9] Review Request: 8075934 Fix some tidy warnings/errors for javax/imageio
Hi, Alexander. The fix looks good. 25.03.15 13:31, alexander stepanov wrote: Hello, Could you please review the fix for https://bugs.openjdk.java.net/browse/JDK-8075934 Webrev: http://cr.openjdk.java.net/~avstepan/8075934/webrev.01/ Just a small HTML markup fix. Thanks, Alexander -- Best regards, Sergey.
Re: [OpenJDK 2D-Dev] [9] Review request for 8023794: [macosx] LCD Rendering hints seems not working without FRACTIONALMETRICS=ON
Hi Andrew, That's a good find re: pow(). In the comment at lines 277-283 I mentioned that there was only a scalar variant of pow(). I wonder if that was a limitation in some early version of GLSL I was using or perhaps some driver bug/restriction. According to all the docs I can find the vector variants have been there all along: https://www.opengl.org/sdk/docs/man4/index.php So now I'm wondering if the slowness was actually due to me trying 3 scalar pow() calls versus one vector pow() call. Oh, aha! I think this explains part of it: 269 * This is the GLSL fragment shader source code for rendering LCD-optimized 270 * text. Do not be frightened; it is much easier to understand than the 271 * equivalent ASM-like fragment program! Looks like I wrote the original version of this shader using the GL_ARB_fragment_program language, which indeed only had a scalar POW instruction (see section 3.11.5.20): https://www.opengl.org/registry/specs/ARB/fragment_program.txt And then I'm guessing that when I rewrote it in more modern GLSL, I didn't notice that pow() supported vectors. Sigh. That 3D texture LUT was a lot of work for a whole lot of nothing. In any case, you might want to rewrite the comment at lines 277-283 to suit the new code. Same goes for the comment at line 315: // gamma adjust the dest color using the invgamma LUT Also, I noticed that the restrictions in OGLContext_IsLCDShaderSupportAvailable() could be loosened since you only need 2 texture units now. Just in the extremely unlikely case that anyone's running this stuff on ancient hardware :) Thanks, Chris Andrew Brygin wrote: Hello Phil, could you please take a look to updated version of the fix? http://cr.openjdk.java.net/~bae/8023794/9/webrev.06/ * OGLTextRenderer.c It was discovered, that in some cases lcd glyphs had visible 'dark border' around. This border appeared due to the gamma correction in lcd shader, which uses lookup on 3D texture instead of using 'pow' routine. The texture is populated with significant granularity (16 points per edge), what is a root cause of rough interpolation of color values. Suggested change is to eliminate the interpolation and use 'pow' routine directly. It gives more accurate color values, and does not introduce significant performance hit (see benchmark summary below). However, this part of the fix affects not only macosx, but any platform where OGL text shader can be used, so it probably worth to split into a separate fix. Summary: lcd-ogl-3Dlookup: Number of tests: 4 Overall average: 42.68027553311743 Best spread: 3.49% variance Worst spread: 29.59% variance (Basis for results comparison) lcd-ogl-pow: Number of tests: 4 Overall average: 42.468941501367084 Best spread: 2.51% variance Worst spread: 29.44% variance Comparison to basis: Best result: 103.28% of basis Worst result: 97.36% of basis Number of wins: 1 Number of ties: 2 Number of losses: 1 Thanks, Andrew
Re: [OpenJDK 2D-Dev] [9] Review request for 8023794: [macosx] LCD Rendering hints seems not working without FRACTIONALMETRICS=ON
Hello Phil, could you please take a look to updated version of the fix? http://cr.openjdk.java.net/~bae/8023794/9/webrev.06/ * CGGlyphImages.m The main part of the recent changes is related to generation of LCD glyph images. With previous version of the fix we produced lcd glyphs which were quite close to jdk6 results if we render dark text on light background (black on white as an extreme case). However, it was discovered that in the case of light text on dark background, our results had significant differences comparing with jdk6. In order to reduce the difference, now we produce two separate glyph images (black-on-white and white-on-black), blend them, and then pass the blended glyph image to text pipes. This approach seems to give an acceptable results for both light and dark cases. * OGLTextRenderer.c It was discovered, that in some cases lcd glyphs had visible 'dark border' around. This border appeared due to the gamma correction in lcd shader, which uses lookup on 3D texture instead of using 'pow' routine. The texture is populated with significant granularity (16 points per edge), what is a root cause of rough interpolation of color values. Suggested change is to eliminate the interpolation and use 'pow' routine directly. It gives more accurate color values, and does not introduce significant performance hit (see benchmark summary below). However, this part of the fix affects not only macosx, but any platform where OGL text shader can be used, so it probably worth to split into a separate fix. Summary: lcd-ogl-3Dlookup: Number of tests: 4 Overall average: 42.68027553311743 Best spread: 3.49% variance Worst spread: 29.59% variance (Basis for results comparison) lcd-ogl-pow: Number of tests: 4 Overall average: 42.468941501367084 Best spread: 2.51% variance Worst spread: 29.44% variance Comparison to basis: Best result: 103.28% of basis Worst result: 97.36% of basis Number of wins: 1 Number of ties: 2 Number of losses: 1 Thanks, Andrew On 12/12/2014 8:11 PM, Andrew Brygin wrote: Hello Phil, at the moment, I failed to get acceptable results for rendering to translucent destination. I am playing with an idea to fall back to grayscale AA on non-opaque pixels (or pixels with opacity less than a certain threshold) directly in the lcd shader program. For now, there is an updated version of the fix which supports lcd text in OGL pipeline for a case of alpha paint and default composite. However, the case of translucent destination is not supported yet, that makes lcd text unavailable in swing apps on macosx. All changes specific for this version are in OGLSurfaceData.java and OGLTextRenderer.c. Rest of the fix is unchanged. http://cr.openjdk.java.net/~bae/8023794/9/webrev.04/ Of course, this version may affect the performance of lcd text rendering, but I have no numbers to estimate the effect at the moment. In the worst case, we can consider to use two different programs: one for opaque paint (in order to preserve performance), and another (like published now) for the case of alpha paint. Please take a look. Thanks, Andrew On 10/25/2014 12:03 AM, Phil Race wrote: On 10/24/2014 10:56 AM, Andrew Brygin wrote: Hello, please take a look to updated version of the fix: http://cr.openjdk.java.net/~bae/8023794/9/webrev.03/ TODOs were removed: * CGLSurfaceData.java the condition for lcd rendering is inherited from OGL surface data, but here we have to remove the check for the transparency, because we always create transparent volatile images for swing backbuffers on macosx in order to support shaped windows. Are you saying its created translucent regardless, or only if its shaped? But if its really translucent/transparent under the text, then IIRC the shaders - and s/w loops - do not properly handle that case. You probably should try drawing LCD text over a fully transparent area to see what happens. * * *CGGlyphImages.m and AWTStrike.m NSException is used to handle invalid (unevaluated) text antialising hint values. We actually have already used NSException to handle memory allocation failure, so this change just makes usage of get/release of primitive arrays a bit more safe. Known differences comparing to apple jdk6: a) we do not interpret TEXT_ANTIALIASING_ON as lcd text. Its should never have meant that, else you would have no way to explicitly request grey scale. b) we do not render lcd text in a case of non-opaque paint. This behavior is common for all cases (software loops, OGL, and D3D), so it seems to deserve a separate bug if we want to handle this case. I think that's another manifestation of the issue mentioned above. -phil.
[OpenJDK 2D-Dev] [9] Review Request: 8075934 Fix some tidy warnings/errors for javax/imageio
Hello, Could you please review the fix for https://bugs.openjdk.java.net/browse/JDK-8075934 Webrev: http://cr.openjdk.java.net/~avstepan/8075934/webrev.01/ Just a small HTML markup fix. Thanks, Alexander
Re: [OpenJDK 2D-Dev] Won't fix https://bugs.openjdk.java.net/browse/JDK-8069312 ??
On Tue, 2015-03-24 at 10:48 -0700, Phil Race wrote: > On 3/24/2015 10:34 AM, Mario Torre wrote: > > On Tue, 2015-03-24 at 10:28 -0700, Phil Race wrote: > >> > i.e is it still JBS but there is some other target release value that > >> should be being used ? > >> > >> It seems that question has been answered in the link Dalibor sent : its > >> "openjdk7u" > >> for the release. > > Yes, this makes totally sense, however the bug was closed as won't fix. > > > > I know I'm failing from my side in adjusting to this new work flow so > > please indulge me, but I would expect the bug to be still open and just > > move from 7-pool to openjdk7u. > > There is a workflow - one that I think is over-used - of creating > backports and > closing them as "not an issue", or "will not fix", as a way of making it > clear > that "we didn't just forget about this release, we made a decision > documented here". > Sometimes they are created and closed like that all in the blink of an eye. > That's not exactly what happened here, since I think there may have been an > intent to fix that was rescinded, but probably it could be considered to > document > that better by its presence than its absence. Ok, I see. I guess what bothers me is the lack of comments on the bug report then, from the external it appears as if someone just unilaterally decided the bug won't be fixed without offering any explanation. I still think that increasing the release flag and add a comment is a better thing to do. > > > > Or will a new backport be createad with openjdk7u as target release? > > * If you pushed to the new openjdk 7 without doing anything, then a new > backport will > be automatically created when you push > * If you create a new one manually, assign to yourself, target it to > openjdk7, then that one > will get marked fixed when you push > * Or, you could re-open the existing one, target it to openjdk7u (and > assign to yourself) > and it will be marked fixed when you push. However I think the workflow > I documented > above will then be messed up. Right, I'll either create a new report or push directly then, I'll try my best to not mess up the workflow :) Cheers, Mario
