Re: [OpenJDK 2D-Dev] Making the OpenGL-Queue-Flusher work concurrently with AWT threads possible? (... the future of the opengl pipeline)
Hi Sergey, It is quite interesting! But it would be good to check what code was > affected by this parallelization. I meant we cannot draw much-much faster > than OGL draw its primitives, so even w/o parallelization if we will call > flush after each fillRect/etc(mimics the in-place rendering on the current > thread), we can speed up something but at the end, we will call OGL on one > thread. > So executing some code in parallel to the actual OGL rendering we speed up > the whole benchmark so much! looks like something(code executed in > parallel) works ineffective. > Basically, all the pipeline-code on the java-side can now be executed in parallel with the actual GL calls: - rasterization of antialiased geometry - span/scanline generation of non-antialiased geometry - queue protocol generation - pipe validation - actual RenderQueue protocol generation - all the locking for each & every primitive So sure, for GPU limited cases this won't help a lot - however, typical java2d is usually CPU limited with tons of very small primitives and many state changes in between. Laurent has done a few benchmarks with mapbench where a ~30% improvement was achieved. Not mindblowing, but also not that bad: https://github.com/bourgesl/jdk/wiki/OpenGL-fix-benchmarks Best regards, Clemens
Re: [OpenJDK 2D-Dev] Making the OpenGL-Queue-Flusher work concurrently with AWT threads possible? (... the future of the opengl pipeline)
Hi Laurent, Thanks for your interest in tuning the pipelines and all the work you did in this regard with Marlin. > I read again the source code and wanted to add metrics like wait_ratio and make some new heuristics to auto tune the buffer capacity. > I was also tempted to rewrite the buffer overflow strategy: swap buffer or use a larger one... > Apparently you made another approach (double buffer) to reduce the wait latency in awt threads... and allow gl thread to run concurently: excellent ! > Could you share your patch to let me study it and try improving it ? Sure, I've forked openjdk at: https://github.com/ceisserer/jdk However please keep in mind this is still very much a proof-of-concept hack done in one day only: - There are some places in native code which do not expect the pointer to the buffer to change after a flush, which are still not modified. - I guess it would be better to return the new RenderBuffer from ensureCapacity and flushnow, instead of relying on callers to re-fetch it after calling those methods - I am not 100% convinced the rewritten locking in the OglRenderQueue flusher thread is correct under all circumstances. This code would need a rewrite/clean-up and error-handling, now that it has to take into account two different operating modes (sync / async flush). As this has to fit into AWT's existing locking scheme, it took me some time to get it working. - there are still rough edges and issues/bugs: when flushNow in OGLBlitLoops.IsoBlit is changed to flush async, I get crashes etc - so some places somewhere are either not aware of the buffer change, or expect certain behaviour I don't know about. (with async flush I get GLX errors and other strange issues). just to be curious, are you running the proprietary nvidia opengl driver? Best regards, Clemens
Re: [OpenJDK 2D-Dev] Making the OpenGL-Queue-Flusher work concurrently with AWT threads possible? (... the future of the opengl pipeline)
Hi Sergey, > The design is a little bit different, the XRender pipeline: > - Many threads which call draw operations and sends them to the XServer > - The XSerever decode the the draw operation and push the commands to the "opengl buffer" > - Some video driver code which decode the ogl buffer and actually draw something > The OGL pipeline > - Many threads call draw operations, and save them in the internal RenderQueue buffer > - One RenderQueue thread decode the the draw operation and push the commands to the "opengl buffer" > - Some video driver code which decode the ogl buffer and actually draw something So in both cases producers (application threads) and the consumer are decoupled via some kind of queue (unix domain socket for x11, in-memory queue for the renderqueue) and in theory could operate concurrently. > I am not sure that it work that way, the only way to block the queuing thread is to call "flushNow()", for other cases it should not be blocked. > The OGLRenderQueue thread however could became blocked every 100ms. Since there is only one RenderQueue+Buffer, the entire queue has to be locked while the commands are processed by the RenderQueue-Thread (via the AWT lock held by the thread which is implicitly calling flushNow triggered by a full buffer). This serverly limits parallelism - you can either process commands (RenderQueue thread) or queue new commands (AWT threads) but not both at the same time. This lead me to the original question - whether this was a necessity caused by structural requirements/limitations or whether it simply hasn't been thought through / implemented. I did some prototyping of a double-buffered renderqueue (no other changes) over the weekend and results are really promising (1x1 fillRect to measure pipeline-overhead, 20x20 aa oval to get an impression data-heavy buffer interaction): Test(graphics.render.tests.fillRect) averaged 1.4837645794468326E7 pixels/sec with !antialias, to VolatileImg(), ident, SrcOver, single, width1, 1x1, !clip, bounce, Default, !xormode, !alphacolor, !extraalpha Test(graphics.render.tests.fillOval) averaged 3.896264839428713E7 pixels/sec with !alphacolor, SrcOver, 20x20, Default, antialias, bounce, !xormode, to VolatileImg(), !clip, width1, !extraalpha, single, ident whereas the original JDK with OGL yielded: Test(graphics.render.tests.fillRect) averaged 5061909.644344761 pixels/sec with single, 1x1, SrcOver, !extraalpha, bounce, Default, !xormode, to VolatileImg(), ident, !alphacolor, width1, !antialias, !clip Test(graphics.render.tests.fillOval) averaged 1.0837940280832347E7 pixels/sec with single, 20x20, SrcOver, !extraalpha, bounce, Default, !xormode, to VolatileImg(), ident, !alphacolor, width1, antialias, !clip and with XRender: Test(graphics.render.tests.fillRect) averaged 2.5252814688096754E7 pixels/sec with ident, to VolatileImg(), 1x1, !clip, !extraalpha, width1, !alphacolor, Default, single, bounce, !antialias, !xormode, SrcOver All test results: Test(graphics.render.tests.fillOval) averaged 2.53725229244114E7 pixels/sec with ident, to VolatileImg(), 20x20, !clip, !extraalpha, width1, !alphacolor, Default, single, bounce, antialias, !xormode, SrcOver To be honest I don't have an explanation why results are *that* good, but on the other hand - XRender is still clearly faster for 1x1 fillRect and not *that* much slower for the maskfills. > There are many ways to improve the OGL pipeline using new OGL feature, but be aware that currently the OGL > pipeline is the only one crossplatform pipeline which works virtually on most of devices(new and old). > A similar architecture issue exists in the d3d pipeline as well, so maybe the Vulkan will better, > it could replace XRender and D3D someday? > > I thought Vulkan superseded the OGL, no? It did and for sure it would be a good/better path forward. On the other hand it would require a new pipeline implementation and not just incremental changes, something hard to do in spare-time ;) Thanks and best regards, Clemens
[OpenJDK 2D-Dev] Making the OpenGL-Queue-Flusher work concurrently with AWT threads possible? (... the future of the opengl pipeline)
Hi, With solid OpenGL support on Linux being ubiquitous these days and the XRender pipeline being a bit of a dead-end (works quite well except MaskBlit/MaskFill/BufferedImageOps), I was looking a bit into the state/performance of the OpenGL pipeline. Specifically why it performs sometimes worse compared to XRender, despite almost all XRender implementations are running on top of OpenGL these days anyway (except proprietary nvidia). 1. One area where XRender is having an advantage is implicit parallelism. While Java is producing X11 protocol, the XServer can concurrently perform the drawing operations running on a different core. Therefore when running some Swing benchmarks with xrender enabled I see java consuming 100% of one core, while the XServer consumes ~50% of another one. With the OpenGL pipeline on the other hand, just one core is fully loaded - despite a similar design (one flusher thread calling into OpenGL, and one or more independent threads queuing drawing operations into a buffer). The reason seems to be the OGLRenderQueue has just one buffer, so either the flusher thread is active or a queuing thread but not both. I wonder, have there been attempts made to introduce double-buffering here and have producers (awt threads) and consumer (queue flusher thread) running concurrently here? 2. Especially MaskFill did perform quite poor in my tests, because drivers are typically not optimized for tiny texture uploads (32x32 coverage masks). Just stubbing out the subTex2D call improved framerate of one benchmark from 100fps to 300fps. I have done some prototyping uploading coverage masks via Shader_Storage_Buffer_Object, but that requires ARB_shader_storage_buffer_object (GL 4.3) as well glBufferStorage (4.4), so effectivly OpenGL-4.4. On the pro side of this approach composition peaked at about 10GB/s with 64x64 mask tiles. Which leads me to the next question: The pipeline currently is written in rather ancient OpenGL-2.x style. Once the need to support old OSX versions is gone, the OpenGL pipeline would not be enabled be default on any platform. Once this is the case, would it be feasable to clean up the code by making use of modern GL and by leaving legacy platforms behind? Modern GL has many improvements in areas where the current pipeline is suffering a bit (overhead of state changes for small drawing operations, etc). Thanks and best regards, Clemens
Re: [OpenJDK 2D-Dev] Fix for 8235904 "Infinite loop when rendering huge lines"
Hi Phil, > Is there a regression test ? > I don't see a noreg- label. Sorry I forgot to mention, the regression test is in the "main" directory of the bugfix: http://cr.openjdk.java.net/~ceisserer/8235904/ Thanks and best regards, Clemens
[OpenJDK 2D-Dev] Fix for 8235904 "Infinite loop when rendering huge lines"
Hi, Please review the fix for bug 8235904 "Infinite loop when rendering huge lines": http://cr.openjdk.java.net/~ceisserer/8235904/webrev.01/ The issue was caused by integer overflow during clipping: While the original native code used int or long depending on the coordinate range, the java-port unconditionally always used ints - which led to the issue. The code was ported to java back then to avoid JNI overhead. A big thanks to Mario Torre for analyzing the issue in detail and bringing it to my attention. Thanks, Clemens
Re: [OpenJDK 2D-Dev] RFR: 8198991: Move Java2D demo to the open repository
Hi, > This moves the Java 2D demo from "closed" to "open". > It is now called "J2Ddemo" to avoid any trademark questions. > This has already been reviewed internally and the images here are OK to > keep. This is awesome news :) Java2Demo is such a handy tool to visually check various features provided by Java2D. Br, Clemens
Re: [OpenJDK 2D-Dev] Initial implementation for batched AA-mask upload for xrender
Hi Sergey, > Thank you for contribution! If there is a chance to implement it soon(at an > early test stage) then it is possible to push this to jdk11 and enable it by > default to expose all possible issues. If no issues will be found, then we > can release it as-is, otherwise we can disable it by default via property. Just to get some idea, how soon is soon? ;) There shouldn't be a lot missing, however clean-ups wouldn't do any harm to the code. Thanks you in advance, Clemens
Re: [OpenJDK 2D-Dev] Initial implementation for batched AA-mask upload for xrender
Hi everybody, I finally got my hands on a 10-year old NVidia GPU (8800GTS) and can confirm Laurent's findings. The proprietary nvidia driver is awesome for the MaskBlit workload, for x11perf -shmput10 it is 32x faster than my kaveri APU solution (despite it has to copy data via PCIe). > Finally, J2DBench results do not show any gain on tested cases. The good news is that actually the Java-process seems to be CPU limited, while the XOrg's CPU consumption is actually lower with deferred (well, throughput is also lower). So the lower throughput seems to have its origin somewhere in client code - and is not caused by the fact that deferred is actually uploading more pixels to the GPU. -> Profiling ;) Does anybody know good statistical profiles these days? I have netbeans a try (worked well in the past) but it seems broken, VisualVM reports all time is spent in SunGraphics2D.fill/draw without providing any further details regarding the call stack. Any recommendations? However even with tuning, chances are small regressions for nvidia+proprietary driver (not the open-source one shipped with linux distributions by default) will remain. All in all, the market share of linux desktops with nvidia gpus running with the proprietary drivers is probably limited to linux-gamers and power-users. For all other drivers, especially glamor based drivers (AMD, Intel, nvidia OSS), issuing larger PutImage requests results in huge gains. And I don't think the situation will change, I tried to lobby for improving this workoad for years, without a lot of success. The only architecture which was ~okish was Intel-SNA, which is now beeing replaced by glamor (which again is performing horrible). As X doesn't provide reliable ways to detect the GPU, we could: * tune it and live with the regression on proprietary nvidia GPUs * try to detect whether the system is running the proprietary nvidia driver (best-guess) disable deferred. Would it be worth the trouble? Best regsrda, Clemens > I dig into d3d code and it has a mask cache (8x4 tiles) so it looks close to > your shm approach. To some degree - however it doesn't batch mask uploads (glTextSubImage2D is called for each tile), but only the resulting blits. > I started tuning the RenderQueue buffer (32k => 4m) and both d3d & ogl gets > better but still 10% slower. I wonder how they would perform with less agressive buffer capacity increase? > Maybe xr suffers also from small buffer capacity (128k)... Unlikely, before the buffer is full we have to flush most of the time because the SHM upload buffer is fully occupied. Without deferred, the internal buffer is 16kb.
Re: [OpenJDK 2D-Dev] [11] Upgrade to Marlin renderer 0.9.1
HI, > As previously mentioned, I increased the RenderQueue buffer (see patchs at > the end) to 4M (instead of 32K) and d3d & opengl get boosted. It is great to see the tile-size improvements reach the other backends too :) In case of the buffer size - I wonder is 4M really needed or was it an arbitrary number (128x increase)? The reason I ask is the overhead of a smaller RenderQueue (the Queue flushes are process-local (actually a thread-switch & wait) and I am a bit concerned about memory consumption of non-graphically intensive applications (+4mb for e.g. a small swing app would be a big deal). Best regards, Clemens
Re: [OpenJDK 2D-Dev] Initial implementation for batched AA-mask upload for xrender
Hi Laurent, Thanks a lot for taking the time to test the deferred xrender pipeline. Especially since the proprietary nvdia driver is the only one of the accelerated xrender implementations I didnt test / benchmark against. > On my linux laptop (i7 + nvidia quadro), xrender is already faster than the > opengl backend (jdk11) on my MapBench tests. > Finally, J2DBench results do not show any gain on tested cases. > SHM is slightly better on nvidia (driver supposed to disable it ?) or > XRBackend / XCB is more efficient with SHM handling. This is really interesting - it seems the proprietary nvidia driver is currently the only driver handling the current xrender operations well. Back in 2009 I've written a standalone C benchmark to stress the types of operations (JXRenderMark) performed by the xrender pipeline and I know the nvidia people had a look at it, great to see this actually turned out to be useful after all. I could live with no performance win on nvidia, but I defintivly would like to avoid regressions. Seems I have to get access to a machine equipped with nvidia gpu and test mapbench there. > Yesterday I looked at the OpenGL backend code and your new XRDeferedBackend > looks very closed to OGLRenderQueue (extends RenderQueue) so you may share > some code about the buffer queue ? > Moreover, OpenGL backend has a queue flusher although XRDeferedBackend has > not ! Exactly, the RendereQueue based pipelines actually buffer their own protocol which they "replay" later from a singel thread, whereas the deferred xrender pipeline directly generates X11 protocol and therefore avoids one level of indirection. So despite the similarities, the actual implementation differs quite a bit. > Does it mean that few buffered commands may be pending ... until the buffer > queue or texture is flushed ? The deferred Xrender pipeline behaves no different than the x11 or the "old" xrender pipeline one in this regard. The self generated protocol is flushed when someone calls into a native Xlib function, by the callback returnSocketCB()l > Here is my first conclusion: > - nvidia GPU (or drivers) are so fast & optimized that the XRender API > overhead is already very small in contrary to intel / AMD CPU that have > either slower GPU or less efficient drivers. > - anybody could test on other discrete GPU or recent CPU ? In this case the overhead is caused by drivers, GPU utilization for most/all of those workloads is typically minor. > Why not larger texture than 256x256 ? > Is it uploaded completely in GPU (compromise) ? or partially ? Uploaded is only the area occupied by mask data. 256x256 is configureable (at least in code), and was a compromise between SHM areas ni-flight and memory use. > Is alignment important (16 ?) in GPU ? ie padding in x / y axis may improve > performance ? > Idem for row interleaving ? is it important ? > Why not pack the tile as an 1D contiguous array ? For ShmPutImage it doesn't matter, for XPutImage this is exactly what the code in PutImage does. > I am a bit lost in how tiles are packed into the SHM_BUFFERS ... and why the > normal XPutImage is more complicated than in XRBackendNative. This is an optimization - since we have to copy the data to the socket anyway, we can use this copy-process to compensate for different scan between the mask-buffer and the width of the uploaded area (therefore data is copied to the socket line-by-line). > - how to pack more efficiently the tiles into larger textures (padding) in x > or XY directions ? use multiple textures (pyramid) ? This is an area which could need improvement. For now tiles are layed out in a row one after another util the remaining buffer-width < tile-width and a next row is started. > PS: I can share (later) my variant of your patch (as I slightly modified it) > to fix typos, debugs ... That would be great. Thanks again & best regards, Clemens
[OpenJDK 2D-Dev] Initial implementation for batched AA-mask upload for xrender
Hi, After achieving huge speedups with Marlin Laurent Bourgès recently proposed increasing the AA tile size of MaskBlit/MaskFill operations. The 128x64 tiles size should help the Xrender pipeline a lot for larger aa shapes. For smaller xrender stays rather slow. To solve this issue am currently working on batching the AA tile mask uploads in the xrender pipeline to improve the performance with antialiasing enabled. Batching can happen regardless of state-changes, so different shapes with different properties can all be uploaded in one batch. Furthermore that batching (resulting in larger uploads) allows for mask upload using XShm (shared memory), reducing the number of data copies and context switches. Initial results seem very promising - beating the current OpenGL implementation by a wide margin: J2DBench, 20x20 ellipse antialiased: XRender + deferred mask upload + XSHM: > Test(graphics.render.tests.fillOval) averaged > 3.436728470039390E7 pixels/sec > with width1, !clip, Default, !alphacolor, ident, > !extraalpha, single, !xormode, antialias, SrcOver, 20x20, bounce, to > VolatileImg(Opaque) XRender + deferred mask upload: > Test(graphics.render.tests.fillOval) averaged > 3.0930638830897704E7 pixels/sec > with width1, !clip, Default, !alphacolor, ident, > !extraalpha, single, !xormode, antialias, SrcOver, 20x20, bounce, to > VolatileImg(Opaque) OpenGL pipeline: > Test(graphics.render.tests.fillOval) averaged > 1.3258861545909312E7 pixels/sec > with Default, !xormode, !extraalpha, single, bounce, > 20x20, to VolatileImg(Opaque), ident, !clip, !alphacolor, antialias, > SrcOver, width1 XRender as-is: > Test(graphics.render.tests.fillOval) averaged > 6031195.796094009 pixels/sec > with !alphacolor, bounce, !extraalpha, !xormode, > antialias, Default, single, ident, SrcOver, 20x20, to > VolatileImg(Opaque), !clip, width1 And a real-world test: MigLayout Swing Benchmark with NimbusLnf, ms for one iteration: XRender-Deferred + SHM: AMD: 850 ms Intel: 1300 ms OpenGL: AMD: 1260 ms Intel: 2580 ms XRender (as is): AMD: 2620 ms Intel: 4690 ms (AMD: AMD Kaveri 7650k / Intel: Intel Core i5 640M ) It is still in prototype state with a few rough edges and a few corner-cases unimplemented (e.g. extra alpha with antialiasing), but should be able to run most workloads: http://93.83.133.214/webrev/ https://sourceforge.net/p/xrender-deferred/code/ref/default/ It is disabled by default, and can be enabled with -Dsun.java2d.xr.deferred=true Shm upload is enabled with deferred and can be disabled with: -Dsun.java2d.xr.shm=false What would be the best way forward? Would this have a chance to get into OpenJDK11 for platforms eith XCB-based Xlib implementations? Keeping in mind the dramatic performance increase, even outperforming the current OpenGL pipeline, I really hope so. Another change I would hope to see is a modification of the maskblit/maskfill interfaces. For now marlin has to rasterize into a byte[] tile, this array is afterwards passed to the pipeline, and the pipeline itself has to copy it again into some internal buffer. With the enhancements described above, I see this copy process already consuming ~5-10% of cpu cycles. Instead the pipeline could provide a ByteBuffer to rasterize into to Marlin, along with information regarding stride/width/etc. Best regards, Clemens Some background regarding the issue / implementation: Since the creation of the xrender java2d backend, I was always bothered how poor it performed with antialiasing enabled. What the xrender backend does in this situation seems not to be that common - the modern drivers basically stall the GPU for every single AA tile (currently 32x32). Pisces was so slow, xservers could consume the tiles more or less at the speed pisces provided it. However with the excellent work on Pisces's successor Marlin (big thanks to Laurent Bourgès), the bottleneck the xrender pipeline presented was more and more evident. One early approach to solve this weakness was to implement the AA primitives using a modified version of Cairo, sending a list of trapezoids to the x-server instead of the AA coverage masks. However this approach has it's own performance issues (and is considered hard to GPU-accelerate) and finally because of the maintenance burden the idea was dropped. The root of all evil is the immediate nature of Java2D: Java2D calls into the backends with 32x32 tiles and expects them to "immediatly" perform a bleding operation with the 32x32px alpha mask provided. In the xrender pipeline, this results in a XPutImage call for uploading the coverage mask immediatly followed by an XRenderComposite call performing the blending. This means: - a lot of traffic on the X11 protocol socket for transferring the mask data -> context switches - a lot of GPU stalls, because the uploaded data from system-memory is immediatly used as input for the GPU operation - a
[OpenJDK 2D-Dev] Proposal: improved AA tile upload for xrender
Re: [OpenJDK 2D-Dev] [10] RFR JDK-8176795: Wrong color drawn when painting translucent colors on volatile images using XRender.
Hi Jay, > 1) When we call Graphics2D.setColor() it uses ArgbPre PixelConverter based on > SurfaceData(here it is XRenderPixMap ArgbPre surface) and converts the color > to pre-multiplied values. Thanks a lot for identifying the issue and working on it. The fix itself looks fine and low-risk. Thanks again, Clemens
Re: [OpenJDK 2D-Dev] Different rounding applied by ProcessPath and ShapeSpanIterator
Hi Jim, > They do operate differently. They aren't meant to be mixed and matched. Thanks a lot for the detailed description of the relationship between SSI and ProcessPath. I did some further digging as to why the xrender pipeline has issues with the test attached to the mentioned bug report - because when I wrote the xrender pipeline the java2d specific code was written with the OpenGL pipeline used sample - which also mixes SSI with ProcessPath. The testcase works fine with D3D/OpenGL, because BufferedRenderPipe has intrinsics for operations that can be lowered to parallelograms. As soon as I replace the rectangle with an arc, the OpenGL pipeline shows alignment issues like xrender. This makes sense, because the code for draw(SunGraphics2D sg2d, Shape s) and fill(SunGraphics2D sg2d, Shape s) is quite compareable between xrender and d3d/ogl. The main culprit seems to be that the test fills with a 3px wide stroke (-> SSI is used) and later calls draw() with a 1px stroke (ProcessPath). If the stroke does not change, SSI or ProcessPath are used consistently for fills and draws - otherwise it can occur that they are "mixed and matched". I have to admit I am unsure how to proceed... Best regards, Clemens
Re: [OpenJDK 2D-Dev] Request for Review: JDK-8162591 All existing gradient paint implementations have issues with coordinates/sizes larger than Short.MAX_VALUE (exactly) on any Linux systems
HI Jim, > All of those conditions are optimized equivalents to getting the > determinant. > > invert() is a destructive call, but since getTransform is documented to > return a copy, that is fine - but getDeterminant() will involve less math to > check the condition... Thanks for the clarification. Please find the latest version of the patch at http://93.83.133.214/8162591/webrev.04/ - where catching the Exception has been replaced with a != 0.0 check against the determinant. Please commit in case no further issues are found. Best regards, Clemens
Re: [OpenJDK 2D-Dev] Request for Review: JDK-8162591 All existing gradient paint implementations have issues with coordinates/sizes larger than Short.MAX_VALUE (exactly) on any Linux systems
Hi Sergey > I guess you can use "getDeterminant()==0" to determine if this transform has > no inverse? Initially I had the same idea, but looking at the implementation of invert(), I found several different conditions leading to an NoninvertibleTransformException. So I decided to keep the call to invert(), as the code-path shouldn't be performance critical anyway. Best regards, Clemens
Re: [OpenJDK 2D-Dev] Request for Review: JDK-8162591 All existing gradient paint implementations have issues with coordinates/sizes larger than Short.MAX_VALUE (exactly) on any Linux systems
Hello again, > OK .. should be part of the patch tho' Just noticed the reg tests are also open-source, until now I thought they are integrated into some Oracle-only internal test collection. Cool! Please find the current version of the patch at: http://93.83.133.214/8162591/webrev.03/ It incorporates all review comments made so far. If no further issues are found, please commit it. Thanks & best regards, Clemens
Re: [OpenJDK 2D-Dev] Request for Review: JDK-8162591 All existing gradient paint implementations have issues with coordinates/sizes larger than Short.MAX_VALUE (exactly) on any Linux systems
Thanks a lot for the feedback. > The fix seems fine .. is there a possibility to have a regression test? > Perhaps based on the code in the bug ? Please find an automated regression test at: http://93.83.133.214/8162591/HugeGradientTest.java > Should we also check the transform? > I guess we already do this in some way, but is it enough? I was not 100% sure myself. As the Paint classes are stateless, there is some overhead involved in checking the transformation in isPaintValid (allocation of an additional AffineTransformation by calling MultipleGradientPaint.getTransform() + additional call to invert() as there is no isInvertible() method). I guess it doesn't matter at all - considering how much else is going on when creating a new GradientPaint. Please find the updated webrev at: http://93.83.133.214/8162591/webrev.02/ Best regards, Clemens
[OpenJDK 2D-Dev] Request for Review: JDK-8162591 All existing gradient paint implementations have issues with coordinates/sizes larger than Short.MAX_VALUE (exactly) on any Linux systems
Hello, The cause of JDK-8162591 is Java2D's gradient implementation allowing for coordinates to be specified using floating point values, whereas XRender is limited to a 16.16 fixed point format - which can therefor easily overflow. This patch checks whether the supplied coordinates can be represented by XRenders fixed-point format, falling back to software otherwise. Please find a fist version of the proposed patch at: http://93.83.133.214/8162591/webrev.01/ Thank you in advance and best regards, Clemens
Re: [OpenJDK 2D-Dev] JDK-8068529 XSync when xrender is turned off
Hi Sergey, Actually it will do this for any blits to the window, because the swing can use different paint strategy(w/ or w/o buffer) or the user can draw something to the window directly via BufferStrategy. Thanks for the clarification. Is there a reason why AWT doesn't explicitly flush and why the flush-after-blit approach was chosen? Thanks best regards, Clemens
Re: [OpenJDK 2D-Dev] JDK-8068529 XSync when xrender is turned off
Hi Jeroen, I'm the submitter of JDK-8068529 https://bugs.openjdk.java.net/browse/JDK-8068529 and I'd like to respond to the comments of this bug Thanks for reporting this issue. In reply to the comments of 2015-01-12: without xrender, XSync is indeed called in X11SurfaceData.c, in function X11SD_GetSharedImage. If you change the code to include a counter and a printf/fflush statement, you can see that XSync is called each time the Java-method paint() is called. This XSync call causes each paint to be displayed on the screen. Great that you were able to find the spot where the sync happens, thanks! It seems it is what I suspected, an implementation artifact. However at least on my system I don't get a completly smooth animation even with the X11 pipeline - although it is a lot better than with the xrender pipeline. The D3D OpenGL pipelines have some code to work around the issue by performing a sync for blits which are equal to the blits of Swing's backbuffer operations. However, I am not very satisfied with the D3d/OpenGL way. X is a network based system where sync/flush-operations are quite expensive and it would actually slow down every direct blit to screen. My plan was to introduce flush-points in AWT to get the content on screen where appropriate instead of guessing in the pipeline which operations could be backbuffer blits. However I am not even sure this is feasable, so it will take some time for to figure out a sane way. Best regards and thanks for your investigations, Clemens
Re: [OpenJDK 2D-Dev] Review request for first part of 8049757
Hi Phil, Do you want me to commit this ? We'd then need a new bug for the next chunk of course .. Please. I'll provide a seperate patch for removing the unused methods you mentioned earlier. Thanks best regards, Clemens
Re: [OpenJDK 2D-Dev] Review request for JDK-8054638 xrender: text drawn after setColor(Color.white) is actually black
Hi Phil, Fix looks good. Is there a way you can check in a regression test along-side the fix else you'll need to add a 'noreg-some reason' label on the JBS bug. Thanks for the review - I'll write a regression test today. - Clemens
Re: [OpenJDK 2D-Dev] Review request for JDK-8054638 xrender: text drawn after setColor(Color.white) is actually black
Hi Phil, Please find the regression test at: http://cr.openjdk.java.net/~ceisserer/8054638/WhiteTextColorTest.java Thanks, Clemens
Re: [OpenJDK 2D-Dev] Review request for JDK-8054638 xrender: text drawn after setColor(Color.white) is actually black
It needs the GPL license (copied from another test to get the right one) and there's one typo : wether - whether Header added, thanks for finding the typo: http://cr.openjdk.java.net/~ceisserer/8054638/WhiteTextColorTest.java Otherwise looks fine. In case no other issues pop up and a second reviewer can be found, please commit. The patch was written against OpenJDK9, however should apply as-is against OpenJDK8. Is there any special backport procedure required or will the fix be applied against both versions at once - as the bug was reported against 8? Thanks, Clemens
[OpenJDK 2D-Dev] Review request for JDK-8054638 xrender: text drawn after setColor(Color.white) is actually black
Hello, Please review my fix for JDK-8054638 xrender: text drawn after setColor(Color.white) is actually black. The XRSolidSrcPict manages a small 1x1 pixmap which is used as source surface for some operations. To avoid redundant fillRect operations when setting it's color, the currently set pixel value is stored in an integer variable and compared against each before the managed pixmap is used. This integer variable was initially set to -1, which corresponds to white - while the actual pixmap had been initialized to black in the constructor - causing white text to be painted black under some circumstances. The fix corrently initializes the variable used for validation to solid black. Thank you in advance, Clemens
Re: [OpenJDK 2D-Dev] Review request for JDK-8054638 xrender: text drawn after setColor(Color.white) is actually black
Sorry, forgot to include the link to the webrev: http://cr.openjdk.java.net/~ceisserer/8054638/webrev.00/ 2014-08-13 7:32 GMT+02:00 Clemens Eisserer linuxhi...@gmail.com: Hello, Please review my fix for JDK-8054638 xrender: text drawn after setColor(Color.white) is actually black. The XRSolidSrcPict manages a small 1x1 pixmap which is used as source surface for some operations. To avoid redundant fillRect operations when setting it's color, the currently set pixel value is stored in an integer variable and compared against each before the managed pixmap is used. This integer variable was initially set to -1, which corresponds to white - while the actual pixmap had been initialized to black in the constructor - causing white text to be painted black under some circumstances. The fix corrently initializes the variable used for validation to solid black. Thank you in advance, Clemens
[OpenJDK 2D-Dev] Review request for first part of 8049757
Hello, Please review my changes for the first patch of a series to come to cleanup the Xrender Java2D pipeline. http://cr.openjdk.java.net/~ceisserer/8049757/webrev.00/ This patch removes support for the Jules rasterizer backend, which relied on a modified version of the native Cairo library. It has never really left experimental state, and with ongoing work to improve pisces performance (I really hope Marlin will make it into OpenJDK9 - https://github.com/bourgesl/marlin-renderer ) and way better 2D drivers than 2008/2009 removing this code reduces the codebase of the Xrender pipeline quite a bit (~ 1000 lines removed in total). Thank you in advance, Clemens
Re: [OpenJDK 2D-Dev] Graphics2D.drawLine() seems to ignore BasicStroke.getEndCap()
Hi Julien, Mageia Linux' bug tracker most likely isn't the best place to file this bug - most distributors simply package OpenJDK and aren't involved in it's development (with redhat being one of the noteworthy exceptions) This bug is reproducible only with the XRender pipeline. In your bug report you mention the issue happens when sun.java2d.x11.X11Renderer.drawLine() is used - which is part of the old X11 pipeline, not the xrender one. With JDK8 the xrender pipeline is enabled by default, so could you please try JDK8 on your platform (there were some changes in the xrender line drawing code between JDK7 and 8, that is why trying JDK8 makes more sense). Best regards, Clemens
Re: [OpenJDK 2D-Dev] Merging marlin pisces rendering engines
Hi, I would like to add that, beside higher performance for rasterization, Marlin's capability to produce tiles larger than 32x32px also improves the performance with the XRender pipeline für larger antialiased shapes a lot. For modern drivers uploading 32x32 small coverage masks is more expensive than uploading the additional empty areas caused by larger masks. Marlin could help to improve the XRender pipeline's weakest spot - in this case the results of the diagonal line (worst case for larger coverage masks) test of J2Bench are shown: graphics.opts.sizes=100: mask256: 3202.226720 (var=33.64%) (100.0%) mask32: 477.4535809 (var=3.01%) (14.91%) stock: 479.0772966 (var=5.82%) (14.96%) At least on this machine the optimal mask size is somewhere between 64^2 and 128^2 pixels. Regards, Clemens
Re: [OpenJDK 2D-Dev] [PATCH] JDK-4627340 : RFE: A way to improve text printing performance for postscript devices (Improved proposal)
Hi Alex, now that my OCA has been processed I would like to bring to attention a proposal that I posted earlier this year to this group. And below a repost of the most recent version of the patch. Glad to hear the OCA process did work out for you :) I don't know if anyone remembers but the idea of the patch is to use embedded Type 3 fonts in favor of glyph vectors in calls to Graphics2D.drawString(). If required I can summarize the whole topic once more. When I first read about your work I immediatly thought what a cool enhancement :) I don't have anything to add from the technical side, I just wanted express my appreciation. Thanks for working on this feature, I hope it will make it into the JDK. Regards, Clemens
Re: [OpenJDK 2D-Dev] request for review: JDK-8028722: XRender: Drawing strings with exactly 254 glyphs causes hangs
Hi Phil, Thanks for the review. Not that it impacts us now but I think the tricky question is can we ever remove the workaround? ie how will we ever know when we are using a fixed xrender lib. I'd like to see core Xrender add a GetVersion() API so we can figure this stuff out at runtime. In this case the decision to leave the workarround in forever is easy - it only causes minimal protocol overhead for larger strings (0.05 extra bytes per 253 characters). In other areas an API to detect the driver / xorg and library versions would have saved a lot of headache. However with wayland/mir/... on the horizon, there is still hope for better opengl drivers in mid-term future :) Thanks, Clemens
[OpenJDK 2D-Dev] request for review: JDK-8028722: XRender: Drawing strings with exactly 254 glyphs causes hangs
Hi, Please review my fix for JDK-8028722: XRender: Drawing strings with exactly 254 glyphs causes hangs available at http://cr.openjdk.java.net/~ceisserer/8028722/webrev.00/ A test is available at: http://cr.openjdk.java.net/~ceisserer/8028722/XRenderElt254TextTest.java Problem description: When drawing strings with a length that is a multiple of 254 characters, the xrender pipeline hangs. This is caused by a bug in libXrender/XRenderCompositeText32 which calculates a wrong request length in this case and passes garbage bytes to the xserver. Fix description: As a workarround, this fix forces a new elt to be used after 253 characters, doing the split exactly two characters before libXrender would do it internally anyway. A patch for libXrender is awaiting review currently. Thank you in advance, Clemens
Re: [OpenJDK 2D-Dev] RFR: Fix for 8027541: Fully transparent jframe becomes black.
Hi Phil, http://cr.openjdk.java.net/~prr/8027541/ In the case where there is an alpha color on the window, the X11 pipeline looks at the color model of the GraphicsConfiguration and upgrades the surface to one with alpha. This makes the Xrender pipeline do the same. SQE test failure, so no regression test needed in this case. Thanks for taking care of this one. The fix looks fine to me. Regards, Clemens
Re: [OpenJDK 2D-Dev] Resurrecting Lauren'ts work on speeding up Pisces
Hi, One wish regarding pisces came into my mind: Currently all AA tile generators feed 32x32 coverage tiles to the pipelines. While this might be a good idea for software-only pipelines due to good cache locality, it is a huge issue for at least the xrender pipeline. Each tile has to be uploaded seperately, which causes context switches and GPU stalls - and I can't make use of the shared memory image extension which should be quite benefitial for larger coverage uploads, It would be great if this fixed size could be made more dynamically - this is an area where I could contribute. Regards, Clemens
Re: [OpenJDK 2D-Dev] AWT Dev [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Hi, PLEASE, PLEASE don't make us wait until JDK9 for something that worked really well in Apple's JDK6. An internal API doesn't mean you can't use it with JDK8, only that it is not part of the default API (which is more or less the same as with Apple's JDK6). However wouldn't it better to wait for an early JDK8 fetaure update, instead of breaking the Image interface with an un-sound design forever? - Clemens
Re: [OpenJDK 2D-Dev] Request for review: Additional fix for JDK-7159455
Hi Jim, Thanks for taking the time to review this in-depth. I guess that also explains why the source coordinates were multiplied by the scale, though that seems a rather odd way to handle these issues. Indeed, the offsets could be also stored as part of the transformation (as it is done for TransformBlit), which would be more elegant. A new question/issue - on line 314, what happens if the case is both quadrant rotated and extra-alpha'd? Either of those will get us into that code block, but also both of them could be true at the same time in which case it looks like you forego the retreival of the EA mask...? In this case a 1x1 mask with EA prepared in advance by XRCompositionManager will be used, returned by xrMgr.getExtraAlphaMask(). However, thinking about it again, it seems to me the code could be simplified a bit more. As soon as a TransformBlit is quadrant rotated, the else-branch (line 321) can handle all cases even with extra-alpha (compositeBlit handles EA in a generic way) - which would make the conditional at line 314 redundant. However, I tended to versimplify this part, so maybe this is something for JDK9 ;) - Clemens
Re: [OpenJDK 2D-Dev] AWT Dev [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Hi Jim, We should be looking to obsolete and delete getScaledInstance(), not giving it a new lease on life with new capabilities... Marking it deprecated would probably be a good idea, at least the JDK7 javadocs don't warn developers in any way about the implications of using getScaledInstance(). Maybe something like getScaledCopy() could be provided as replacement (I've never seen code where the in-sync feature of getScaledInstance was used intentionally). - Clemens
Re: [OpenJDK 2D-Dev] Review request for 8024767: [TEST] need test to cover JDK-7189452
Hi Alexander, Please see the updated webrev: http://cr.openjdk.java.net/~bae/8024767/webrev.01/ The fix looks fine to me. Thanks for creating the test-case :) - Clemens
Re: [OpenJDK 2D-Dev] request for review: xrender pipeline ignores center-coordinates of RadialGradient
Hi, Ok, this starts to become embarrassing ... sorry for the review request spam recently. I was stressed that my changes would be submitted too late, so I submitted as soon as I had a fix for an isolated issue. During extensive testing I've found a few other issues within the xrender pipeline paint support. Please review the latest (and from my side last) revision of the patch: http://cr.openjdk.java.net/~ceisserer/fix13/webrev.03/ Additional changes/fixes compared to webrev.02 are: 1. The xrender pipeline accelerated LinearRGB color model gradients, although xrender (from what I found out) only supports sRGB. The pipeline now falls back to software loops when a LinearRGB gradient is requested. (Test: http://cr.openjdk.java.net/~ceisserer/fix13/LinearColorSpaceGradientTest.java ) 2. An AffineTransform can be specified when creating a linear or radial gradient paint. This transform was not applied correctly. The fix makes use of the same mechanisms introduced for TexturePaint's transform support introduced in webrev.02 and does away with special transform handling previously performed at gradient construction time. (Test: http://cr.openjdk.java.net/~ceisserer/fix13/GradientTransformTest.java ) 3. The interpolation value set for a TexturePaint at creation time was never changed. Therefore, switching to bilinear interpolation is ignored in case the TexturePaint was created with the rendering hint set to nearest neighbour. There is already is a mechanism for re-validating source-surface properties, so this has been extended to revalidate the interpolation setting too. This issue is also present in the OpenGL pipeline (didn't test d3d). (Test: http://cr.openjdk.java.net/~ceisserer/fix13/TexturePaintInterpolationTest.java ) Thanks, Clemens
Re: [OpenJDK 2D-Dev] request for review: xrender pipeline ignores center-coordinates of RadialGradient
Please wait with the commit, there are a few other issues hidden in the horrors of the xrender pipeline's gradient support. 2013/10/19 Andrew Brygin andrew.bry...@oracle.com: Hello Clemens, the fix looks fine to me. Thanks, Andrew On 10/18/2013 8:59 PM, Clemens Eisserer wrote: Please find a more extensive fix at: http://cr.openjdk.java.net/~ceisserer/fix13/webrev.02/ In addition to the fixes in webrev.00, this one also fixes the TexturePaint issues reported by Phil. With this patch the TransformedPaintTest is rendered correctly. The problem with TexturePaint was basically the same as with RadialGradientPaint, the initial transform is overwritten by the currently set Graphic2D's transformation. The solution was to additionally store this transformation (includes scale and translation set by the TexturePaint's Rectangle2D-parameter). Thanks, Clemens
[OpenJDK 2D-Dev] request for review: JDK-7082809 : xrender artifacts caused by different interpretation of SRC operator
Please review my fix for bug JDK-7082809 : xrender artifacts caused by different interpretation of SRC operator located at: http://cr.openjdk.java.net/~ceisserer/7082809/webrev.05/ A manual test-case is available at: http://cr.openjdk.java.net/~ceisserer/7082809/CompositeTest.java Problem description: There are a few differences between the composition operators of Java2D and xrender. One difference is e.g. that for SRC the destination is never read inside the composition rectangle, which affects MaskFills and text rendering (where the glyph geometry is stored inside small mask-images). The xrender pipeline ignored most of those differences for now and focussed on correctness and performance with SRC_OVER. Beside 7082809, this patch also fixes the TranTest2 and AlphaCompositeTest JCK tests as well as a issue with older versions of IntelliJ. Fix description: This patch tries to correct this sloppiness by either falling back to software-loops for uncommon operations which haven't been implemented in a correct way, or to implement work-arrounds for common-cases which do not map 1:1 to xrender. Thanks, Clemens
Re: [OpenJDK 2D-Dev] request for review: JDK-7082809 : xrender artifacts caused by different interpretation of SRC operator
Hi Andrew, a copyright header is missed in the new file XRSolidSrcPict.java. Could you please add it? Done. Please find the updated webrev at: http://cr.openjdk.java.net/~ceisserer/7082809/webrev.06/ Thanks, Clemens
Re: [OpenJDK 2D-Dev] Request for review: Additional fix for JDK-7159455
Another revision was required, available at: http://cr.openjdk.java.net/~ceisserer/fix12/webrev.04/ When the destination-area is quadratic, no geometry is stored inside the mask and therefore the composition-window is used for clipping and it's size has to be rounded properly. In cases where it is not quadrant transformed, the code makes now sure no pixels are clipped away unintentionally. Thanks for your patience, Clemens
[OpenJDK 2D-Dev] request for review: xrender pipeline ignores center-coordinates of RadialGradient
Hi, Please review my fix for the issue described below at: http://cr.openjdk.java.net/~ceisserer/fix13/webrev.00/ Problem description: Phil found an issue with TransformedPaintTest, where the center point of radial gradients was stuck at (0,0) with the xrender-pipeline enabled. The center-point was indeed set to (0,0) and offset of the center-point was applied using the source transform. However, when the source transformation of the gradient surface was re-validated later on (XRSurfaceData.validateAsSource()) this translation is lost again. Fix description: This patch sets the center-point now explicitly. It also cleans up the API a bit (there were two completly unused Point2D-objects) and removes unnescessary/dead code. Thanks, Clemens
Re: [OpenJDK 2D-Dev] Request for review: Additional fix for JDK-7159455
Hi, Please find an updated webrev with your suggestions at: http://cr.openjdk.java.net/~ceisserer/fix12/webrev.01/ Am I reading this right. You are saying that it is used in a mixture of cases and some of those will support full transforms? Exactly. What I was suggesting is that the adjust method can do more optimal work if the transform is quadrant-rotated and so it needs to know that information as well. I'm not suggesting changing the technique for determining the type of rotation, Done. If you prefer I could also write a single-if version, which would be more pretty but also contain some duplicate code. This would depend on the semantics of the draw operation. I believe that other places in our other pipelines are consistent with round-down semantics and so this pipeline should be consistent. This may be worth writing a test that slowly translates an image to see when it jumps pixels, especially if it can be automated, so that we can keep all of the pipelines in synch. I can confirm the round-down bahaviour at least for the software-loops. Once I did a test slowly scaling an image, but jump-points were different between the ogl/d3d, xrender and software pipelines. I agree that floor/ceil sounds more correct for this case. Perhaps we need a boolean for the adjust method? Or is there a case where the caller may use it in different ways and doesn't know which way is appropriate before it calls the method? No there isn't, so I implemented it this way ;) Thanks, Clemens
Re: [OpenJDK 2D-Dev] Request for review: Additional fix for JDK-7159455
As usual, seconds after sending a review request I somewhere find another issue - sorry. Please instead of webrev.01, review http://cr.openjdk.java.net/~ceisserer/fix12/webrev.02/ In the previous version, I assumed not using a mask means a quadrant-rotated transform, which is not correct as the mask can also be ommited in any case when nearest neighbour interpolation is used. Thanks, Clemens
[OpenJDK 2D-Dev] Request for review: Additional fix for JDK-7159455
Hi, As Jim pointed out, my earlier fix for bug JDK-7159455 wasn't entirely correct as it allowed some shear-transformations to pass a check for a fast-path although they should have been rejected. He proposed a way of using the capabilities already provided by AffineTransform, which is correct, shorter and most likely more efficient (thanks!). Please find the follow-up fix for JDK-7159455 at: http://cr.openjdk.java.net/~ceisserer/fix12/webrev.00/ Thanks, Clemens
Re: [OpenJDK 2D-Dev] Fix suggestion: Black boxes arround glyphs with SrcIn and Src/SrcIn+EA [OGL, D3D]
Hi Jim, You should be able to use the same code as SRC for CLEAR, but you are rejecting that operation. And, if the destination is opaque then SRC_IN is identical to SRC, but you are rejecting that as well. Perhaps the lower layers are ignoring ISCOPY and seeing the SRC_IN or CLEAR and using different code, but they should vector to the SRC code in those cases rather than rejecting them at the validate stage...? Thanks for the clarification that this was intended as is. Unfourtunately I don't know enough of the inner workings of the OGL/D3D pipelines to fix those paths myself, so for now I'll file a bug and submit the jtreg-test along with it. Perhaps I'll have a look at it when the worst xrender-bugs are ironed out... Thanks, Clemens
[OpenJDK 2D-Dev] Fix suggestion: Black boxes arround glyphs with SrcIn and Src/SrcIn+EA [OGL, D3D]
Hi, Please review my fix-suggestion for the problem described below available at: http://cr.openjdk.java.net/~ceisserer/fix11/webrev.03/ A testcase is available at: http://cr.openjdk.java.net/~ceisserer/fix11/TextCompTest.java Problem description: When rendering Text with AlphaComposite.SrcIn and Src/SrcIn with EA, the accelerated D3D/OGL pipelines render black boxes arround individual glyphs. As far as I can see, this is caused by an invalid check during pipeline validation: /* CompositeType.SrcNoEa (any color) */ -(sg2d.compositeState = SunGraphics2D.COMP_ISCOPY - sg2d.paintState = SunGraphics2D.PAINT_ALPHACOLOR) From my understanding the check should allow for using the accelerated text pipeline for AlphaComposite.SRC when a color is used as source. However, as is the check also accepts the composition rules CLEAR, SRC, SRCIN regardless of extra alpha. Fix description: The fix makes the check strikter to explicitly check for comp-rule==Src and the currently set extra-alpha == 1. It only fixes the issue for the OpenGL pipeline, but the changes should apply 1:1 for the D3D pipeline. Thanks, Clemens
Re: [OpenJDK 2D-Dev] Request for review: JDK-7159455 : Nimbus scrollbar rendering glitch with xrender enabled on i945GM
Hi Jim, Apologies as this is an old bug, but I just looked at the fix and it seems to me that it will allow shearing transforms to pass because the tests are incomplete. The 0or180 flag only tests if the horizontal lines stay horizontal, but they could shift relative to each other in increasing Y which means a shear is present. Similar, the 90or270 flags only verify that vertical lines stay vertical, but allow a shear in X. Thanks a lot for pointing that out, I'll take care about it. - Clemens
Re: [OpenJDK 2D-Dev] RFR: 8024343: Change different color with the The XOR alternation color combobox, the color of the image can not shown immediately.
Hi Vadim, The fix itself looks fine to me - thanks for taking care about this. - Clemens PS: what is the reason for this bug is confidential? Please review a fix for this bug: https://bugs.openjdk.java.net/browse/JDK-8024343
[OpenJDK 2D-Dev] Handling of premultication in the D3D OGL pipelines
Hi, I am currently testing compatibility of the xrender pipeline with different composition operations, and I noticed for AlphaComposite.SRC the D3D and OGL pipelines store pre-multiplied colors in surfaces without an alpha-channel. For example the following code results in a black rectangle, instead of a red one when rendering to a BufferedImage of type INT_RGB: ((Graphics2D) g).setComposite(AlphaComposite.Src); g.setColor(new Color(255, 0, 0, 2)); g.fillRect(10, 10, 100, 100); Is this intentional or should it be considered a bug? Thanks, Clemens
Re: [OpenJDK 2D-Dev] request for review: JDK-8007386 On physical machine (video card is Intel Q45)the case of text is blank.
Hi Phil, Shouldn't most of this be inside an ifdef _linux_ ? Solaris (currently) even MacOS will compile this code. Thanks for the pointer, I completely forgot about OSX (solaris should work as-is). Please find an updated version at: http://cr.openjdk.java.net/~ceisserer/8007386/webrev.02/ - Clemens
Re: [OpenJDK 2D-Dev] Review request for http://cr.openjdk.java.net/~ceisserer/fix10/
Hi Phil, the webrev and test need a little clean-up : adding the @test and @bug tag + GPL Please find a corrected version at http://cr.openjdk.java.net/~ceisserer/8024895/webrev.01/ along with a (hopefully) jtreg conformant testcase at http://cr.openjdk.java.net/~ceisserer/8024895/EABlitTest.java Other than that the fix is fine although without looking at bit more broadly I'm unsure how this cache works in a MT scenario. It is guarded by the AWT-lock, but you're right - it is not benefitial (but doesn't hurt either) under some circumstances. Thanks, Clemens
[OpenJDK 2D-Dev] Review request for http://cr.openjdk.java.net/~ceisserer/fix10/
Hi, Please review my fix at http://cr.openjdk.java.net/~ceisserer/fix10/ I haven't created a an official bug-id, because due to the migration to JIRA those user reports don't seem to be directly useable anyway. Problem description: For transformed blits the xrender pipeline uses caching-logic to be able to use the same mask again when geometry doesn't change. However, that logic didn't take into account changes of the extra alpha value. Fix description: The pipeline now also stores and validates against the extra-alpha value which was used the last time the mask was prepared. Also available is a small regression test, please feel free to modify or discard it. Thanks, Clemens
Re: [OpenJDK 2D-Dev] Request for review: JDK-7159455 : Nimbus scrollbar rendering glitch with xrender enabled on i945GM
the fix looks fine to me. Thanks. Please commit. Regards, Clemens
[OpenJDK 2D-Dev] request for review: 9006475: xrender: improve performance of small fillRect operations
Hi, Please review my patch for 9006475 at: http://cr.openjdk.java.net/~ceisserer/9006475/webrev.00/ Before this patch we were calling the native method XRenderRectanglesNative() even in the case that there was only single rectangle to be rendered (which is the case for Graphics.fillRect). As XRenderRectanglesNative() uses GetPrimitiveArrayCritical, this caused high overhead for small fillRect() oerations. With this patch we call renderRectangle instead, which passes the reactangle's coordinates down using JNI function parameters. On my private Laptop this improved throughput of 1x1 fillRect calls by a factor of 3. Thanks, Clemens
[OpenJDK 2D-Dev] request for review: JDK-7179526 : xrender : closed/sun/java2d/volatileImage/LineClipTest.java failed since jdk8b36
Hello, Please review my fix for: JDK-7179526 : xrender : closed/sun/java2d/volatileImage/LineClipTest.java failed since jdk8b36 at http://cr.openjdk.java.net/~ceisserer/7179526/webrev.00/ Problem description: Lines weren't rendered in a consistent way when they were clipped. The issue originated from clipping being previously separated from line rasterization. Fix description: 1. After a few failed attempts, I decided to port the native line rendering algorithms from sun/java2d/loop/DrawLine.c and sun/java2d/loop/LoopMacros.h to Java. The original source consisted of quite a number of interdependent macros, I tried to split the code into smaller methods in XRDrawLine. 2. Also included is a small low-risk micro-optimization, by providing a GrowableRectArray.pushRectValues()-method, replacing the rather generically implemented MaskTile.addRect() method. This results in a 10-20% speedup for workloads consisting of many small rectangles, as this code is very often called. 3. Another change is a clean-up/removal of long-time unused member variables in MaskTileManager.java, from dates where line rendering was implemented in a different way. I could create separate patch-sets for the clean-up and the micro-optimization, as you prefer. However, I consider both no-risk. Thanks, Clemens PS: Performance for line-rendering workloads improved quite a bit: Options common across all tests: graphics.opts.xormode=false graphics.render.opts.paint=single graphics.opts.renderhint=Default graphics.opts.alpharule=SrcOver graphics.opts.extraalpha=false graphics.render.opts.alphacolor=false global.dest=VolatileImg graphics.render.opts.antialias=false graphics.opts.clip=false graphics.render.opts.stroke=width1 graphics.opts.transform=ident graphics.opts.anim=2 graphics.render.tests.drawLine,graphics.opts.sizes=20: baseline: 23234.32805 (var=18.95%) (100.0%) patch: 26042.31972 (var=6.28%) (112.09%) graphics.render.tests.drawLine,graphics.opts.sizes=250: baseline: 27573.53165 (var=30.54%) (100.0%) patch: 30530.53302 (var=7.46%) (110.72%) graphics.render.tests.drawLineHoriz,graphics.opts.sizes=20: baseline: 93574.96039 (var=21.09%) (100.0%) patch: 103011.66924 (var=3.92%) (110.08%) graphics.render.tests.drawLineHoriz,graphics.opts.sizes=250: baseline: 1129462.56050 (var=15.02%) (100.0%) patch: 1291397.82057 (var=1.53%) (114.34%) graphics.render.tests.drawOval,graphics.opts.sizes=20: baseline: 20958.84885 (var=18.79%) (100.0%) patch: 23018.13843 (var=3.44%) (109.83%) graphics.render.tests.drawOval,graphics.opts.sizes=250: baseline: 15263.36513 (var=19.22%) (100.0%) patch: 33692.11899 (var=1.29%) (220.74%) graphics.render.tests.drawPoly,graphics.opts.sizes=20: baseline: 33479.49456 (var=18.25%) (100.0%) patch: 37251.33485 (var=3.36%) (111.27%) graphics.render.tests.drawPoly,graphics.opts.sizes=250: baseline: 28618.54379 (var=22.98%) (100.0%) patch: 42345.48575 (var=4.03%) (147.97%) graphics.render.tests.shape.drawCubic,graphics.opts.sizes=20: baseline: 10687.11472 (var=13.72%) (100.0%) patch: 10538.90697 (var=33.91%) (98.61%) graphics.render.tests.shape.drawCubic,graphics.opts.sizes=250: baseline: 8458.942632 (var=18.26%) (100.0%) patch: 16690.96833 (var=2.67%) (197.32%) Summary: baseline: Number of tests: 10 Overall average: 139131.1690314054 Best spread: 13.72% variance Worst spread: 30.54% variance (Basis for results comparison) patch: Number of tests: 10 Overall average: 161451.9295901127 Best spread: 1.29% variance Worst spread: 33.91% variance Comparison to basis: Best result: 220.74% of basis Worst result: 98.61% of basis Number of wins: 9 Number of ties: 1 Number of losses: 0
Re: [OpenJDK 2D-Dev] Defect 7032904(XRender: Java2Demo) remains
Hi Andrew, Thanks for your feedback. Do you know which versions of libXrender are supported? We can check the version at configure time: $ pkg-config --modversion renderproto 0.11.1 and disable the extension at compile-time. This is especially true with 6 where, IIRC, it's a patched-in option which is enabled by default, not part of upstream OpenJDK. Version 0.9.3, which was released in early 2007 contains the fix (it's xrender not renderproto). Disabling it at configure time would be great for distribution builds, unfortunately it does not solve the situation for the oracle binaries. Basing it on kernel version seems very dodgy to me. I change my kernel version all the time and it's completely unrelated to the version of Xrender I'm running. I am not 100% satisfied with the proposal either, however the arguments which made me think this isn't such a bad idea are: 1. The main problem are old, supported LTS releases (like RHEL 5 or SLES 10). The chance of someone running a kernel newer than 2.6.32 with libXrender0.9.3 (false negative) are almost zero, while false positives (older kernel, newer libXrender) are handled by the old X11 pipeline without any serious drawbacks. Personally, I don't expect many users running one of those really old distributions updating to a kernel version which is almost 3 years newer than their libXrender. 2. The current code already inspects pkg-config files at run-time (works on RHEL 5), the kernel-version check would only take place when this check fails. Do you still think its a bad idea? Regards, Clemens
Re: [OpenJDK 2D-Dev] Defect 7032904(XRender: Java2Demo) remains
Hi Frank, Recently our team discovered defect 7032904(XRender: Java2Demo : Infinite loop in Java_sun_java2d_loops_MaskBlit_MaskBlit on OEL 5.6 x64) still exists in latest JDK (7u21) on SLES10SP4. It can be easily reproduced by running SwingSet2 with Nimbus LAF. The issue is also seen in Java 8. Can anybody look into it? Thanks for reporting the issue, I'll have a look at it. Most likely the code which is detecting the libXrender package-info version doesn't find the package-info files at the expected place and conservatively keeps the xrender pipeline enabled. The root problem still remains, we can not reliably detect the version of libXrender library used without targeting and testing every possibly affected distribution separately. As the versions of kernel and libXrender usually stay quite coherent for problematic distributions (old kernel == affected libXrender) and it is very easy to query the version of the linux-kernel currently running, I would propose to further restrict use of the xrender pipeline to systems running = Linux-2.6.32, which is currently the oldest LTS kernel still supported. 2.6.32 was released in Dec. 2009. Hopefully this will not only avoid running into the libXrender-bug, but also avoid many driver-bugs caused by old and outdated drivers for the local use-case. In case of false positives it reverts back to the X11 backend instead, which in my opinion is the right thing to do in the event of uncertainty. Regards, Clemens PS: RHEL 5.5/5.6 runs linux-2.6.18 SLES-10 runs linux-2.6.16. Ubuntu 10.04 LTS (and higher) and Debian squeeze (6.0) both use Linux-2.6.32 and therefore would be supported.
Re: [OpenJDK 2D-Dev] sun.java2D.Pisces renderer Performance and Memory enhancements
Hi Laurent, thanks for having some interest for my efforts ! As I got almost no feedback, I felt quite disappointed and was thinking that improving pisces was not important ... Glad to see work is ongoing to improve pisces's performance :) I had a look at the patch just to be curious (I don't have reviewer status), but to be honest I had troubles finding the relevant parts. Having not followed the discussion that closely, it was almost impossible for me to extract the real modifications from boilerplate/refactoring as your patch weights almost 3000 modified lines. I am just an intrested observer without any official state, yet personally I would prefer smaller pieces with clear description/title. However, I wouldn't want to cause you additional work and it's just a single opinion. Thanks for working on pisces! Regards, Clemens
[OpenJDK 2D-Dev] D3D pipeline not working with latest intel-drivers on sandy bridge
Hi, While performance testing to compare the different pipelines I noticed the D3D pipeline doesn't work (Couldn't enable D3D pipeline) with Intel's latest drivers on Sandy Bridge running Windows7. As modern intel GPUs are feature-wise quite on par with real discrete graphic cards, and I am able to run the OpenGL pipeline, I wonder wether its a known bug or if the hw/driver combination has been blacklisted? Thanks, Clemens
Re: [OpenJDK 2D-Dev] Please review patch for 7105461
Hi Jim, Sorry it took so long - could you please have a look at http://cr.openjdk.java.net/~ceisserer/7105461/webrev.02 It does now protect against integer overflow, using the Region.clipAdd. Thanks, Clemens 2012/4/17 Jim Graham james.gra...@oracle.com: This code doesn't protect against integer overflow. We don't protect against it in many places, but it couldn't hurt to get in the habit. I think the Region code has some methods that do safe addition of integers with simple limit clipping that would work fine for rectangle dimensions. (JDK8 will be introducing new methods in Math for unsigned results and exact non-overflowing integer results as well, but that would complicate any backports to JDK7...) ...jim On 4/13/12 9:46 AM, Clemens Eisserer wrote: Hi, Please take a look at the patch for bug 7105461, located at http://cr.openjdk.java.net/~ceisserer/7105461/webrev.00/ The problem was caused by Swing calling drawLine/fillRect with coordinates outside the valid X11 coordinate space. I took the same approach of the original X11 pipeline to simply clamp the corrdinates to the min/max allowed value although its not enterly correct - as it doesn't adjust width/height in case it clamps x/y - triggered for exmaple by the following call: g.fillRect(-32868, 0, 32968, 10); Should I take care of this special case, or is it ok to handle it the same way the X11 pipeline does? Thanks, Clemens
Re: [OpenJDK 2D-Dev] request for review: 7188093
Hi, Automated test will require taking screenshot and comparing results, I'm not sure if this mechanism works good. It should be possible to render to a VolatileImage, which allows direct readback without taking a screenshot. - Clemens
Re: [OpenJDK 2D-Dev] Xrender bicubic interpolation
Hi, Unfourtunatly the situation is worse than I expected, currently xrender doesn't seem to support anything better than billiniar interpolation. Because alias-names where used by the pipeline it didn't turn up sooner (as they are guaranteed to be available, whereas the concrete filter-implementation-name may change from platform to platform). Seems at least for now we have to fallback for bicubic, I'll prepare a patch. Thanks for bringing up this issue! - Clemens 2012/9/3 Vadim Pakhnushev vadim.pakhnus...@oracle.com: Hi Clemens, We've found a bug in Xrender pipeline when setting interpolation rendering hint to bicubic by calling g2d.setRenderingHint(RenderingHints.KEY_INTERPOLATION, RenderingHints.VALUE_INTERPOLATION_BILINEAR) doesn't work as expected. In fact bicubic interpolation seems to fall back to bilinear, which can be clearly seen in the attached screenshot. I've attached the test, there are 3 images rendered with up scaling with various interpolation settings, upper 3 are rendered to the screen, lower 3 to the BufferedImage which works as the reference. With -Dsun.java2d.Xrender=True the bicubic interpolated image is in fact interpolated using bilinear filter. Could you please take a look? Thanks, Vadim
Re: [OpenJDK 2D-Dev] Please review patch for 7150134
Hi Jim, For a complex clip it is not enough to test the two endpoints so I don't think this fix will work for a line that spans between two portions of a non-rectangular clip over a region that is outside the clip... This case is handled by the native clip, so it would just be a missed optimization. Would you like me to include a check for isRetcangular? Thanks, Clemens
Re: [OpenJDK 2D-Dev] Please review patch for 7105461
Hi Phil, Thanks for taking a look. Fix looks reasonable and good to have a fix for this! I do think it would be safer to be better than the X11 pipeline if possible I reworked the original patch a bit and put the checks at a higher level, based on the following two assumptions: - Clipping with the compClip will avoid coordinate overflow all the time - The only place where we get unclipped corrdinates is drawLine and fillRect, all other primitives go through the DrawHandler, which will only receive clipped segments anyway. Lines are already handled by the last patch (OOM), so this one only focuses on fillRetc. http://cr.openjdk.java.net/~ceisserer/7105461/webrev.01/ I also removed a synchronized which served no purpose. Thanks, Clemens
[OpenJDK 2D-Dev] Please review patch for 7150134
Hi Jim, I don't understand. You are checking to see if the line is inside the clip, but your test will think that something is inside the clip when it is not so the fast path code will be used for lines outside the clip. If the case can handle lines outside the clip then why test at all? The xrender pipeline uses bresenham to transform diagonal lines into a lists of rectangles, without clipping it was possible with large line-coordinates to cause an OOM because of the huge amount of rectangles generated. The corner-case of a complex clip where both line end-points are contained would cause unnecessary rectangles to be generated, but still only a few thanks to the component clip. The unnecessary rects would be clipped away by the XServer later. - Clemens
Re: [OpenJDK 2D-Dev] Please review patch for 7150134
Hi Phil, Looks fine should solve the problem. Thanks. Could you please push the patch? Perhaps it would have been nice to calculate the interpolation with the clip but then you'd need to pay attention to whether it was a complex clip or a simple rectangular one and it may be better to do the delegation that you are doing now ... Diagonal lines are quite a sad story with xrender anyway - I tried to keep it simple. For horizonal/vertical lines (which can be reduced to a fillRect) it should have been a win, however. Thanks, Clemens -phil. On 4/5/2012 5:49 AM, Clemens Eisserer wrote: Hi, Please take a look at the patch for bug 7150134, located at http://cr.openjdk.java.net/~ceisserer/7150134/ The problem was caused by a fast-path in drawLine() which didn't take clipping into account, therefor huge lines that would have been clipped away caused OOMs in xrender's bresenham code. Because the fast-path provides nice speedups (e.g. 300% for 20px horizontal lines measured with J2DBench) I don't want to sacrifice it entirely. This fix only takes the fast-path in case the line is completly un-clipped, otherwise it lets generic Java2D code handle it: http://cr.openjdk.java.net/~ceisserer/7150134/ Thanks, Clemens
[OpenJDK 2D-Dev] Please review patch for 7150134
Hi, Please take a look at the patch for bug 7150134, located at http://cr.openjdk.java.net/~ceisserer/7150134/ The problem was caused by a fast-path in drawLine() which didn't take clipping into account, therefor huge lines that would have been clipped away caused OOMs in xrender's bresenham code. Because the fast-path provides nice speedups (e.g. 300% for 20px horizontal lines measured with J2DBench) I don't want to sacrifice it entirely. This fix only takes the fast-path in case the line is completly un-clipped, otherwise it lets generic Java2D code handle it: http://cr.openjdk.java.net/~ceisserer/7150134/ Thanks, Clemens
Re: [OpenJDK 2D-Dev] Please review patch for 7150134
Hi Mario,
The patch looks good to me.
Thanks for taking a look.
Just wondering what is the impact of those two checks:
+ if (compClip.contains(transX1, transY1)
+ compClip.contains(transX2, transY2)) {
for the most common cases?
I wasn't able to measure any difference using J2Bench, out of
curiousity I had a look at Region.contains().
In the common case with a rectangular clip (bands==null), it should
just execute these few comparisons:
if (x lox || x = hix || y loy || y = hiy) return false;
if (bands == null) return true;
Thanks again, Clemens
Re: [OpenJDK 2D-Dev] Turn on Xrender by default in JDK 8
Hi Phil, I think we should turn on Xrender by default in these early days in JDK 8 http://cr.openjdk.java.net/~prr/7077423/ I feared that day would come ... great :) I know of two more-than-minor bugs, however I haven't had the time to file reports: - Problems with SRC + alpha values when rendering to surfaces without alpha, this causes IntelliJ's editor to look garbled. - Somewhere coordinate overflows happen with very large JTables. I don't know if its ok to enable it anyway. I think the only problems may be in buggy drivers, but turning it on will give us a chance to test and resolve remaining issues, if any :) Currently shipping drivers are really all working very well, what could cause problems are probably archaic installations still running early versions of EXA or even XAA. Thanks, Clemens
Re: [OpenJDK 2D-Dev] Question about SurfaceData lock functions
Hi Jim, I'm pretty sure that BufImgSurfaceData does not lock the toolkit lock. I think only the X11SD needs that as it needs to serialize access to X11 and it may need to read back pixels or send pixels via Xlib. Thanks for the clarification. That should help scalability a lot for my use-case :) - Clemens
[OpenJDK 2D-Dev] Question about SurfaceData lock functions
Hi, Just to be sure, Is it allowed/possible to use a per-surface locking structure for SurfaceData's lock/unlock functions to improve concurrency? All OpenJDK pipeline implementations seem to lock the toolkit lock, so maybe the software routines expect the toolkit lock to be locked. How do fallbacks work with the OpenGL pipeline? I just realized it doesn't implement the native lock/unlock/getrasinfo functions, and getRaster() seems to be unimplemented also. Thanks, Clemens
Re: [OpenJDK 2D-Dev] OpenGL Pipeline
Hi Mario, Unfourtunatly the OpenGL pipeline suffers a lot from broken drivers. Drivers seem to be optimized a lot for the game-case, where a frame is repainted over and over again. I remember I read somewhere in nvidia's release-notes that some bugs related to the opengl backend have been fixed in a recent nvidia driver version, maybe a driver update helps? Enabling it with -Dsun.java2d.xrender=True doesn't produce any output, although things appear to be faster (it may be a psychological thing though, didn't do any benchmark :) When True is written with a capital 'T', you should get something like: XRender pipeline enabled lg
Re: [OpenJDK 2D-Dev] OpenGL Pipeline
Forgot to mention, the version included in openjdk6 is really old - I recommend JDK7. - Clemens 2011/6/7 Clemens Eisserer linuxhi...@gmail.com: Hi Mario, Unfourtunatly the OpenGL pipeline suffers a lot from broken drivers. Drivers seem to be optimized a lot for the game-case, where a frame is repainted over and over again. I remember I read somewhere in nvidia's release-notes that some bugs related to the opengl backend have been fixed in a recent nvidia driver version, maybe a driver update helps? Enabling it with -Dsun.java2d.xrender=True doesn't produce any output, although things appear to be faster (it may be a psychological thing though, didn't do any benchmark :) When True is written with a capital 'T', you should get something like: XRender pipeline enabled lg
Re: [OpenJDK 2D-Dev] AWT Dev Cross-Platform print dialog ignores Window's default printer setting?
HI again, I'll try to diagnose it a bit more, maybe I can get an idea whats going wrong there. But at least I have confirmation that it is supposed to honor the default printer setting :) Ok, its a bug in JasperReports. It queries the list of available printers, and sets the first one - effectivly disabling the default-printer detection :/ Thanks, Clemens
Re: [OpenJDK 2D-Dev] AWT Dev Cross-Platform print dialog ignores Window's default printer setting?
Hi Phil, And (forgive me for being nit picky), this is really more of a question for one of the forums on using the APIs rather than the openjdk ones for working on the implementation. That's my standard reminder on this to be consistent. I posted it on the forums about a week before writing to the list (even in the Java2D subforum ;) ) but got zero repsonse. Sorry, I didn't mean to be intrusive. But if I knew the answer off hand, I'd give it, but I don't. We just query the default printer and so the cross platform dialog should show whatever the native dialog does. Maybe there's a locale issue I am not aware of. I'll try to diagnose it a bit more, maybe I can get an idea whats going wrong there. But at least I have confirmation that it is supposed to honor the default printer setting :) Thanks, Clemens
Re: [OpenJDK 2D-Dev] CR 7029934 : Xrender: Text is truncated with 64 bit Linux JRE
Hi Phil, The problem was caused by the assumption that sizeof(int) == sizeof(XID), however Xlib uses unsigned long for XIDs, even if XIDs are defined to be 32-bit on wire. According to the xorg mailing list ists safe to store those values in 4-byte variables, the only case which needs attention are arrays. In this case the problem was I passed a Glyph* to XRenderCompositeText, which however expected a unsigned int*. Both are 4bytes wide on ia32, but Glyph is 8 bytes on amd64. XRenderFreeGlyphs was also affected. - Clemens PS: I can't access information for 7029934 - which seems to be rather common for many bug-numbers I see floating arround on the mailing lists. Any idea why? 2011/3/22 Clemens Eisserer linuxhi...@gmail.com: Hi Phil, I'll have a look at this one soon. I also hope to fix the bug causing intellij's editor to be completly unuseable before jdk7's freeze phase - however I guess I'll need a little help for that. Just remembered that I promies results some time ago, sorry, I'll send it soon. Its just, university is giving me a really hard time. Starting with 6th April everything will be far more relaxed =) Thanks, Clemens 2011/3/22 Phil Race philip.r...@oracle.com: Clemens, Have you tried 64 bit Linux JRE? SQE reports, and I have verified, text is truncated But only on Linux. Solaris is OK. BW, greyscale, LCD are all the same. -phil.. *Change Request ID*: 7029934 *Synopsis*: Xrender: Text is truncated with 64 bit Linux JRE === *Description* java version 1.7.0-ea Java(TM) SE Runtime Environment (build 1.7.0-ea-b134) Java HotSpot(TM) 64-Bit Server VM (build 21.0-b04, mixed mode) Platform: OEL6 2.6.32-100.28.5.el6.x86_64 #1 SMP Wed Feb 2 18:40:23 EST 2011 x86_64 x86_64 x86_64 GNU/Linux Problem: When launch Java2Demo in OEL6 machine, the text partially shows in the application. It only happens in 64 bits java. I tried 32 bits java, the demo looks fine. Please see attached screen shot for detail. How to reproduce the problem: 1, Launch the demo by using jdk1.7.0-b134 ever early versions(like b120) 2, When the demo GUI comes up, you can see all text in frame shows only partially [root@dhcp-santaclara22-2fl-west-10-132-182-109 linux-x64]# bin/java -jar -Dsun.java2d.xrender=True demo/jfc/Java2D/*.jar XRender pipeline enabled Xrender: INFO: Jules library not installed. You can use the OEL6 machine to reproduce this problem, I add machine info is in comments *** (#1 of 1): 2011-03-22 16:41:39 GMT+00:00 tao.t.zh...@oracle.com *** Last Edit: 2011-03-22 17:15:42 GMT+00:00 tao.t.zh...@oracle.com === *Evaluation* = Looks bad. Seems like all text rendering that goes via Xrender is truncated. Looks like the first half of any given drawString is visible and the rest is missing. Its not specific to Java2Demo. Its in SwingSet2 and in applet demos. It seems to have been this way since the first Xrender build (b97) and its not just OEL 6. Its also Ubuntu 10.04, so probably all 64 bit Linuxes. But 64 bit Solaris on Intel/AMD is fine! I reproduced this displaying remotely from an OEL 6 box and an Ubuntu box with Solaris 11 x86 as the display server, so its presumably something on the client (application) end. The code is all the same so maybe its something to do with the compilation. *** (#1 of 1): 2011-03-22 20:37:53 GMT+00:00 philip.r...@oracle.com jdk.patch Description: Binary data
[OpenJDK 2D-Dev] XShmGetImage with image's drawable's size causes BadMatch
Hi,
I've recently encountered some BadMatch errors triggered by fallbacks.
Those are caused by reading outside of surface bounds using the cached
ShmPixmap.
Testcase and patch attached.
Running with -Dsun.awt.noisyerrorhandler=True the testcase causes a BadMatch:
Xerror BadMatch (invalid parameter attributes), XID 517, ser# 215
Major opcode 139 //SHM
Minor opcode 4 //GetImage
If a X11SDOps structure belongs to a pixmap I can directly use
pmWidth/Height, but for windows I have to query the attributes
width/height.
Is there a better way to get window's width/height, maybe through some
of AWT's structures?
- Clemens
--- old/src/solaris/native/sun/java2d/x11/X11SurfaceData.c 2010-11-13 18:32:30.688184052 +0100
+++ new/src/solaris/native/sun/java2d/x11/X11SurfaceData.c 2010-11-13 18:32:30.354433458 +0100
@@ -595,11 +595,11 @@
}
XImage* X11SD_GetSharedImage(X11SDOps *xsdo, jint width, jint height,
- jboolean readBits)
+ jint maxWidth, jint maxHeight, jboolean readBits)
{
XImage * retImage = NULL;
if (cachedXImage != NULL
-X11SD_CachedXImageFits(width, height, xsdo-depth, readBits)) {
+X11SD_CachedXImageFits(width, height, maxWidth, maxHeight, xsdo-depth, readBits)) {
/* sync so previous data gets flushed */
XSync(awt_display, False);
retImage = cachedXImage;
@@ -728,8 +728,8 @@
* it must be close enough to avoid excessive reading from the screen;
* otherwise it should just be at least the size requested.
*/
-jboolean X11SD_CachedXImageFits(jint width, jint height, jint depth,
-jboolean readBits)
+jboolean X11SD_CachedXImageFits(jint width, jint height, jint maxWidth,
+jint maxHeight, jint depth, jboolean readBits)
{
/* we assume here that the cached image exists */
jint imgWidth = cachedXImage-width;
@@ -747,10 +747,13 @@
return JNI_TRUE;
}
-if ((imgWidth width + 64) (imgHeight height + 64)) {
+if ((imgWidth width + 64) (imgHeight height + 64)
+ imgWidth = maxWidth imgHeight = maxHeight) {
/* Cached image's width/height shouldn't be more than 64 pixels
* larger than requested, because the region in XShmGetImage
* can't be specified and we don't want to read too much.
+ * Furthermore it has to be smaller than maxWidth/Height
+ * so drawables are not read out of bounds.
*/
return JNI_TRUE;
}
@@ -1295,7 +1298,7 @@
SurfaceDataBounds *bounds,
jint lockFlags)
{
-int x, y, w, h;
+int x, y, w, h, maxWidth, maxHeight;
int scan;
XImage * img = NULL;
Drawable drawable;
@@ -1311,10 +1314,25 @@
#ifdef MITSHM
if (useMitShmExt == CAN_USE_MITSHM) {
-if (xsdo-isPixmap readBits) {
+ if (xsdo-isPixmap) {
+ if(readBits) {
X11SD_PuntPixmap(xsdo, w, h);
-}
-img = X11SD_GetSharedImage(xsdo, w, h, readBits);
+ }
+
+ maxWidth = xsdo-pmWidth;
+ maxHeight = xsdo-pmHeight;
+} else {
+ XWindowAttributes winAttr;
+ if(XGetWindowAttributes(awt_display, (Window) xsdo-drawable, winAttr) != 0) {
+ maxWidth = winAttr.width;
+ maxHeight = winAttr.height;
+ }
+ }
+
+ maxWidth -= x;
+ maxHeight -= y;
+
+img = X11SD_GetSharedImage(xsdo, w, h, maxWidth, maxHeight, readBits);
}
#endif /* MITSHM */
drawable = xsdo-drawable;
ShmGetOOBTest.java
Description: Binary data
[OpenJDK 2D-Dev] Java's definition of the SRC operator
Hi, Some users reported problems with the IntelliJ Idea's editor when running with xrender enabled. It turned out that there are some differences between how Java and xrender interpret the SRC operator. Is the general rule, that SRC behaves like SRC_OVER when antialiasing is enabled? Are there some special cases that need to be taken care of? Thanks, Clemens
Re: [OpenJDK 2D-Dev] Java's definition of the SRC operator
Hi Jim, blendresult = PORTER_DUFF(rule, rendercolor, dstcolor, extraalpha) // For SRC, blendresult = rendercolor modulated by extra alpha storedresult = INTERP(dstcolor, blendresult, aacoverage) // For full aa coverage, storedresult = blendresult The only part of this that could possibly be interpreted as behaving like SRC_OVER would be the second INTERP and it depends on the aa coverage, not on the alpha of the colors involved. Is that what they were talking about? Thanks for the detailed explanation. What confuses me is that pixels wich don't have full AA coverage take the src's alpha into the calculation. Shouldn't the following operations both yield the same result when rendered to an opaque destination? /*SRC*/ g2d.setComposite(AlphaComposite.getInstance(AlphaComposite.SRC)); g2d.setRenderingHint(RenderingHints.KEY_ANTIALIASING,RenderingHints.VALUE_ANTIALIAS_ON); g2d.setColor(new Color(255, 0, 0, 1)); g2d.drawLine(100, 120, 200, 220); /*SRC_OVER*/ g2d.setComposite(AlphaComposite.getInstance(AlphaComposite.SRC_OVER)); g2d.setColor(Color.red); g2d.drawLine(100, 140, 200, 240); Thanks, Clemens
[OpenJDK 2D-Dev] Strange text rendering with SRC + Extra Alpha
Hi, While playing arround with the SRC composition rule I found what seems to be a bug. When rendering text with an AlphaComposite(SRC, EA) I get garbage when enabling subpixel antialising: g2d.setComposite(AlphaComposite.getInstance(AlphaComposite.SRC, 0.01f)); g2d.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING, RenderingHints.VALUE_TEXT_ANTIALIAS_LCD_VBGR); g2d.setRenderingHint(RenderingHints.KEY_ANTIALIASING,RenderingHints.VALUE_ANTIALIAS_ON); g2d.setColor(Color.black); g2d.drawString(This text is painted with black color, 50, 50); If I comment out the KEY_ANTIALIASING hint, which shouldn't have any effect on text rendering, extra alpha is ignored but the text is rendered correctly. Should I file a but? Thanks, Clemens
Re: [OpenJDK 2D-Dev] Strange text rendering with SRC + Extra Alpha
Hi Denis, It's interesting to note that this doesn't happen on OpenJDK, and on closed java the subpixel hint is not necessary to reproduce this. Simply using VALUE_ANTIALIAS_ON and the SRC composite is enough. I have to enable lcd text antialising to get the described result, however I see it with OpenJDK6 and JDKb99 from oracle. Another thing which looks odd when I disable LCD antialiasing is pixels that lay on straight lines seem to ignore extra alpha as shown in the screenshot attached. - Clemens attachment: textaa.png
Re: [OpenJDK 2D-Dev] CR 6974985 Redispatched, P3 java/classes_2d Jave2Demo threw exceptions when xrender enabled in OEL5.5
Hi Phil,
It doesn't sound right to me that this behaviour is an implementation detail
of X.org.
Developers need to know that either it does or it doesn't. Were you given a
good rationale?
The explanation was that Pixmaps are reference counted, and therefor
stay alive if a XRender-picture points to it. A window is not
reference counted, and therefor all its resources are destroyed
immediatly. GC's are not affected because they are not bound to a
drawable, thats why it hasn't been a problem for the JDK to free GCs
later than the Window.
I find this explanation fishy, but the general recommendation was to
free Pictures before the drawable in any case.
The patch attached does free the picture before destroying the window,
however casting surfaceData to XRSurfaceData in XWindow.java is ugly -
and the patch assumes nobody calls XDestroyWindow for a
SurfaceData-Drawable except destroy(). Suggestions welcome ;)
With that patch I get rid of all XErrors and also can't reproduce the
XDnD-Exception anymore.
I don't see how that helped on its own. If the Picture is implicitly freed
by XDestroyWindow then
xsdo-xrPic is still going to be != None .. or am I overlooking something?
My initial analysis was wrong - somehow I thought the problem is
solved, but it wasn't.
Thanks Clemens
PS: Is there some java-side equivalent in SurfaceData for X11SD_Dispose?
I have a GC on the java side with no references on the native side and
if possible would like to free it at dispose time without writing C
code.
--- old/make/sun/awt/mapfile-mawt-vers 2010-09-24 22:25:13.453158179 +0200
+++ new/make/sun/awt/mapfile-mawt-vers 2010-09-24 22:25:13.060159227 +0200
@@ -424,6 +424,7 @@
Java_sun_java2d_xr_XRSurfaceData_initXRPicture;
Java_sun_java2d_xr_XRSurfaceData_initIDs;
Java_sun_java2d_xr_XRSurfaceData_XRInitSurface;
+ Java_sun_java2d_xr_XRSurfaceData_freeXSDOPicture;
Java_sun_java2d_xr_XRBackendNative_initIDs;
Java_sun_java2d_xr_XIDGenerator_bufferXIDs;
Java_sun_java2d_xr_XRBackendNative_freeGC;
--- old/make/sun/xawt/mapfile-vers 2010-09-24 22:25:14.103157831 +0200
+++ new/make/sun/xawt/mapfile-vers 2010-09-24 22:25:13.850167398 +0200
@@ -375,6 +375,7 @@
Java_sun_java2d_xr_XRSurfaceData_initXRPicture;
Java_sun_java2d_xr_XRSurfaceData_initIDs;
Java_sun_java2d_xr_XRSurfaceData_XRInitSurface;
+ Java_sun_java2d_xr_XRSurfaceData_freeXSDOPicture;
Java_sun_java2d_xr_XRBackendNative_initIDs;
Java_sun_java2d_xr_XRBackendNative_freeGC;
Java_sun_java2d_xr_XRBackendNative_createGC;
--- old/src/solaris/classes/sun/awt/X11/XWindow.java 2010-09-24 22:25:14.690157342 +0200
+++ new/src/solaris/classes/sun/awt/X11/XWindow.java 2010-09-24 22:25:14.421408215 +0200
@@ -44,6 +44,8 @@
import sun.java2d.SunGraphics2D;
import sun.java2d.SurfaceData;
+import sun.java2d.xr.XRSurfaceData;
+
public class XWindow extends XBaseWindow implements X11ComponentPeer {
private static PlatformLogger log = PlatformLogger.getLogger(sun.awt.X11.XWindow);
private static PlatformLogger insLog = PlatformLogger.getLogger(sun.awt.X11.insets.XWindow);
@@ -1383,6 +1385,18 @@
destroy();
}
+void destroy() {
+XToolkit.awtLock();
+try {
+ if(surfaceData != null surfaceData instanceof XRSurfaceData) {
+ ((XRSurfaceData) surfaceData).freeSurfacePicture();
+ }
+ super.destroy();
+} finally {
+XToolkit.awtUnlock();
+}
+}
+
public Point getLocationOnScreen() {
synchronized (target.getTreeLock()) {
Component comp = target;
--- old/src/solaris/classes/sun/java2d/xr/XRSurfaceData.java 2010-09-24 22:25:15.309407237 +0200
+++ new/src/solaris/classes/sun/java2d/xr/XRSurfaceData.java 2010-09-24 22:25:15.025418970 +0200
@@ -55,6 +55,8 @@
native void initXRPicture(long xsdo, int pictForm);
+native void freeXSDOPicture(long xsdo);
+
public static final String DESC_BYTE_A8_X11 = Byte A8 Pixmap;
public static final String DESC_INT_RGB_X11 = Integer RGB Pixmap;
public static final String DESC_INT_ARGB_X11 = Integer ARGB-Pre Pixmap;
@@ -396,6 +398,10 @@
int validatedRepeat = XRUtils.RepeatNone;
int validatedFilter = XRUtils.FAST;
+public void freeSurfacePicture() {
+ freeXSDOPicture(getNativeOps());
+}
+
/**
* Validates an XRSurfaceData when used as source. Note that the clip is
* applied when used as source as well as destination.
--- old/src/solaris/native/sun/java2d/x11/X11SurfaceData.c 2010-09-24 22:25:15.937407516 +0200
+++ new/src/solaris/native/sun/java2d/x11/X11SurfaceData.c 2010-09-24 22:25:15.665159018 +0200
@@ -375,6 +375,12 @@
AWT_LOCK();
xsdo-invalid = JNI_TRUE;
+
+if(xsdo-xrPic != None) {
+ XRenderFreePicture(awt_display, xsdo-xrPic);
+ xsdo-xrPic = None;
+}
+
if (xsdo-isPixmap == JNI_TRUE xsdo-drawable != 0) {
#ifdef MITSHM
if
Re: [OpenJDK 2D-Dev] Another Xrender bug ..
Hi again, Any thoughts on this one that just got submitted a few minutes ago? I'm guessing you don't have a SPARC System with OpenSolaris handy so I'm just asking for ideas here. The only thing I can think of is the copy code which copies/converts the GlyphInfo image-data into the stream submitted to X, in XRGlyphCacheEntry.writePixelData(). I guess the pixel-data I get from the GlyphInfo structure is the same, maybe access using Unsafe is somehow broken or shifted by a byte? For now I assume a fixed oder when uploading the glyph-data (didn't find any docs about it), could it be different on that machine? Thanks, Clemens -phil. *Change Request ID*: 6980378 *Synopsis*: LCD subpixel text has yellow background with Xrender enabled in jdk1.7.0-ea-b106 on SPARC. === *Description* Java Version: 1.7.0-ea-b106 Platform: OpenSolaris 5.11 snv_146 Problems: When launch Java2Demo/SwingSet2 with Xrender=True or true in b106, you can see all letters or fonts are marked with yellow color or yellow background, but disable xrender, it looks normal, no yellow background, please see attached files for xrender enabled and disabled screen shots for details. File this bug to make sure that this is not a problem caused by xrender enabled. It can be reproducible as alway by the following command: bin/java -jar -Dsun.java2d.xrender=True demo/jfc/Java2D/*.jar We have a OpenSolaris 5.11 snv_146 newly installed in lab, the host name is listed in comments if you want to reproduce it. The framebuffer in the machine is XVR-100. *** (#1 of 1): 2010-08-26 21:55:21 GMT+00:00 tao.t.zh...@oracle.com *** Last Edit: 2010-08-26 22:46:10 GMT+00:00 tao.t.zh...@oracle.com === *Evaluation* = The background that is yellow follows the rectangular shape of the individual glyphs. Otherwise everything appears correct. Seeing this only on SPARC. At this time there's only one system here with the right hardware/software combination to run XRender on SPARC so it could be an Xserver side problem. Its not been seen on Solaris on Intel and it also doesn't occur on software buffered images, only on XRender surfaces.
Re: [OpenJDK 2D-Dev] 2 small xrender fixes
Hi Phil, I see the problem now. I looked at what T2K (the rasteriser) reports, but in the non-T2K glue code we then make an adjustment in the case of LCD glyphs which simply should never have been applied in the case of an empty image. It has never affected any of our code as I think we only truly use the width (not the same as advance) during blitting and the check for a null image stops us before that's an issue. Great you were able to reproduce it. With XRender the width is stored when uploading the glyph server-side, and somehow X didn't check for an empty glyph. I'll file a bug and fix this. Thanks :) I'll do some further validation on the locking stuff, I am still not really sure were these jules crashes come from :-/ - Clemens
Re: [OpenJDK 2D-Dev] 2 small xrender fixes
Hi Phil, Strange, running your test I get width=-1 when uploading as well as rendering the glyph - and the pipeline basically just reads the native GlyphInfo structure by using Unsafe. I'll have to think about it a bit more .. Anything I could to to help to track that down? Thanks, Clemens
[OpenJDK 2D-Dev] 2 small xrender fixes
Hi, I would like to get two (very small) changes to the xrender-pipeline in: 1. http://93.83.133.214/ignore-whitespace-width.zip deals with the fact that the closed-source font rasterizer seems to generate width=65535 for whitespaces, which resuts in software fallbacks. Granted with height=0 rendering should be completly omited by the X server, but as it seems it isnt. 2. http://93.83.133.214/synchronize-native-cairo-ops.zip synchronize-native-cairo-ops synchronizes access to Jules's native functions. There seems to be a race somewhere in cairo and I wasn't able to hunt it down, so synchronizing is the best I can do for now. Do I need to create two different bug-reports? Is OpenJDK Bugzilla enough?
Re: [OpenJDK 2D-Dev] Integration of the xrender pipeline rewite
By the way ... The Jules cairo pisces replacement obviously isn't all that useful without the backend .. is it likely the required cairo modifications would ever make it into linux distros ? If not I'm not sure its worth putting the java glue code into openjdk. Wouldn't it be possible to put the backend itself into OpenJDK? There's already a quite a lot of 3rd party code in OpenJDK, would one more hurt that much? Even for MaskBlit's, it usually does a lot better than pisces. (with some exceptions that can be fixed for sure) Thanks, Clemens
Re: [OpenJDK 2D-Dev] Integration of the xrender pipeline rewite
Hi Andrew, Which then has to be maintained with regard to security updates, etc. IcedTea pulls out the in-tree libjpeg, libpng and zlib libraries and uses the system ones to avoid this issue. Can't it be made to work with the system cairo? For XRender it would require rendering to an intermediate A8 surface, for MaskBlit/Fills it would require pre-allocating a large image, and copying the tiles. It could in both cases, but it would mean loosing a good deal of performance. Hmm, for now it builds and runs quite well. If at one point it doesn't anymore, if nobody fixes it there's no big deal in reverting to pisces. I know its an ugly situation, but whats the difference if I had written a rasterizer completly from scratch and asked for integration. That would also require maintenance. Ok, it probably would be way less code ;) - Clemens
Re: [OpenJDK 2D-Dev] Integration of the xrender pipeline rewite
Hi Phil, Thanks for taking the time to review the code. I uploaded the new/resulting webrev to: http://93.83.133.214/webrev-xrender-jules-0.0.2.zip The Jules cairo pisces replacement obviously isn't all that useful without the backend .. is it likely the required cairo modifications would ever make it into linux distros ? If not I'm not sure its worth putting the java glue code into openjdk. No, the modifications won't make it into cairo, I already talked with the cairo maintainers about that. However the only difference between using a forked version of cairo and writing a new rasterizer from scratch is that the cairo-code can't be under Sun's SCA. At least when looking at benchmarks, Jules does well when used as AATileGenerator, and it shines when used in conjunction with the XRender pipeline for antialiased rendering. XRender implementations usually dont handle uploading those small AA tile masks generated by Ductus well. Dimitri's suggestion is to have build code in the make files that looks for the files and builds them if they are there. This would make it easier for people to get those sources, drop them in and experiment I really like that idea. My hope is that Jules will be shipped by distributors, so this would fit my goal best :) Aside from that I think the biggest concern I have is to understand the changes to glyph caching. I don't think its necessary to change it at least for any existing pipeline. I think that's why you only call StrikeCache.addGlyphDisposeListener in XRGlyphCache.java Right, only XRGlyphCache uses the DisposeListener. So far so good except that I don't see how the requirements for xrender are that much different from opengl - we also there have a limited size texture and need to manage the contents of that. I'd like to understand better the inadequacies of that code or mismatch in the requirements. Near as I can tell its that you are counting pixels whereas we count cells? I already had a private glyph-cache implementation in the old xrender pipeline. As far as I can remember this is because the pipeline doesn't handle fallbacks because exhausted cache space (this is done by the xserver). The pipeline depends on the fact that all glyphs used in a DrawGlyphList() call are cached. The pixel-couting isn't a strict limit, only a hint how much should stay cached. All the low-level details are handled by the xserver anyway. One of the goals of the new pipeline was to write as much as possible in java (partly to avoid JNI, partly to make maintenance and porting easier). So we have a new per glyph variable managed which could use some documentation on its meaning. I think its something like 1 means this glyph has a hardware cached copy and its freeing is managed by the usual 2D disposer code managed by GC and reference queues. A value of 0 means its either unaccelerated (and so has no cellInfos) or we want to free this in a different way. Exactly. If managed=0 and CellInfo!=0 the entry is managed by the XRender glyph cache. The drawback is that all code which uses AccelGlyphCache has to set managed=1. The whole thing is about getting notified when GlyphInfo's are deleted by a GC, so that the native cache can be flused as well. I'll add a comment. So in freetypeScaler.c lines 785 : you set managed = 0. Looks like this maybe just initialisation to non-garbage although 0 which is part of the trigger to make the existing glyph caching code know to use your new disposer code instead. Yes I see you also made a change in AccelGlypheCache.c so that (eg) OpenGL accelerated glyphs will set managed = 1 The Xrender code doesn't use this function. Yes Then in sunFont.c we call RemoveAllCellInfos only for the managed ones (managed != 0) Yes, because for the XRender glyph's are java objects, so nothing to free here. Next in StrikeCache.java if you detect an Xrender glyph (implied by managed==0 and there being a cached cell pointer) its added to a list to be disposed by Xrender's listener or its delegate. Right. BTW I see you inserted this managed byte into the GlyphInfo struct in fontscalerdefs.h at a point where the compiler probably had some padding. So it probably doesn't matter if its there even if we don't use this field on windows, as it isn't increasing the size of the struct. Perhaps you could add a comment to that effect. Yes, I made sure it uses padding and the structures doesn't grow. I'll add a comment on this. I suppose I'll find out later why you changed 95 struct _CacheCellInfo *cellInfo; to void* I am not really used to C, I mis-use a pointer as integer anyway so I guess changing the type to (void*) doesn't make any sence. The XRender pipeline uses it to store an integer, which is used to map a native GlyphInfo structure to a XRGlyphCache java object using a HashMap. - in src/share/classes/sun/font/StrikeCache.java I'd create disposeListeners() only for the xrender pipe. Then there's
Re: [OpenJDK 2D-Dev] Integration of the xrender pipeline rewite
Hello, I've merged the xrender pipeline with master, please take a look at the webrev: http://93.83.133.214/webrev-xrender-jules-0.0.1.zip It would be great to get the review-process rolling for the critical parts like StrikeCache.java or sunFont.c It also includes a preview of Jules, a cairo based RenderingEngine implementation: Because of its complex build-system and the need for a modified version of cairo I've seperate the native components and build them indepent from OpenJDK - falling back to pisces when loading fails. The native part can be found at: http://93.83.133.214/jules-0.0.1.tar.gz Simply copy the resulting libjules.so into lib/i386, and activate it on the command-line: -Dsun.java2d.renderer=sun.java2d.jules.JulesRenderingEngine Jules is more or less proof-of-concept, especially the native code is ugly, full of dirty assumptions and its probably not 64-bit clean. (Although it should get a major performance boost, in the case it works ;) ). However it runs Java2Demo quite well, and usually is a good deal faster than pisces even when rendering to software-surfaces. Known problems: - Some clipping problems when rendering to software surfaces - Paints get wrong transformation when rendering to XRender surfaces Thanks, Clemens PS: Is there an easy way to apply a webrev to a local repository?
[OpenJDK 2D-Dev] Howto install a custom rendering engine?
Hi, I am trying to install my own RenderingEngine implementation, however I am not accustomed to the ServiceLoader concept: ServiceLoaderRenderingEngine reLoader = ServiceLoader.loadInstalled(RenderingEngine.class); the ServiceLoader doesn't e.g. contain my CairoRenderingEngine. Would it be enough to simply create a META-INF directory with a single file in it, similar to what has been done for pisces? Is there any way to influence the iteration-order? If possible I would prefer my RendeingEngine to be tried before Pisces when its mature and complete, at least it should be configureable without passing the cmd-line parameter every time. Whats the purpose of the ServiceLoader, wouldn't it be possible to simply load the class specified on the command-line directly? Thank you in advance, Clemens
Re: [OpenJDK 2D-Dev] Integration of the xrender pipeline rewite
Hi Andrew, How close is what you have to what we have in IcedTea? I'm thinking it might be a good idea to have your XRender work in the IcedTea forest rather than as patches in IcedTea7. That would give you a clean webrev against the latest JDK7 code. Its a complete rewrite and doesn't share much code with what is in IcedTea. I'll try to update to the lastest JDK7 code from the mercurial repos later today, after backing that stuff up ;) - Clemens
Re: [OpenJDK 2D-Dev] Integration of the xrender pipeline rewite
Hi, Thanks for all your work, integrating and maintaining the pipeline. So I should just ditch what we have in IcedTea7 and use this webrev? I had to reroll all the patches for b74 anyway so get rid of that maintenance headache would be nice. Yes, in the long run I hope this code will replace what is currently in IcedTea. However the rewrite has more impact on independent parts (like the StrikeCache changes), so even with the pipeline disabled it could break some stuff. Thats why some review would be great... - Clemens
Re: [OpenJDK 2D-Dev] Integration of the xrender pipeline rewite
Hi Phil,
Thanks for taking a quick look.
Seems the webrev is in a rather chaotic state, I'll try to clean it
up a bit to make review easier.
I have a very few superficial comments from my quick look at a few files
that are *changed*.
X11SurfaceData and the glyph caching changes need a slightly longer
look. I do note we'd need to make some closed source changes to
match some of it.
Yes, the glyph caching changes and the modifications to X11SurfaceData
are basically the modifications I think would be
best included first.
Most of the other stuff hasn't even been reviewed by myself.
1) src/share/classes/sun/font/FontManager.java
shows that this isn't synced with Roman's refactoring.
Shouldn't be a big deal though as you only change
one line and I don't think even that line should be changed
where you changed an array size from 20 to 50 :
private static CompositeFont [] compFonts = new CompositeFont[50];
I can't guess how x render defines new logical fonts.
Argh, I thought I did a sync with the 2d-repo. I really don't like Mercurial ;)
I remember I did that change, because without it I gout
ArrayIndexOutOfBoundsExceptions on my system,
independent from my changes. I just tried to resize the Array without
looking a lot why or what, and it worked.
Sorry, I forgot about that change, its not ment to go upstream.
2) src/share/classes/sun/java2d/pipe/AAShapePipe.java
imports sun.java2d.pisces.*
but doesn't (and shouldn't) do this directly, and
also has new CairoRenderingEngine(); //.getInstance();
I'm not sure what's intended here. Obviously it
shouldn't directly instantiate that here, but also
I don't even see this class in the webrev.
Sorry, this is from my experiments replacing Pisces with Cairo:
http://linuxhippy.blogspot.com/2009/09/ductus-vs-cairo-vs-pisces.html
Because it requires a slightly modified version of cairo, which I
guess would cause build-system troubles, I don't think its ready for
upstream too.
3) src/share/classes/sun/java2d/pipe/RenderQueue.java
the size change from 32000 to 12000 probably should be explained :
75 private static final int BUFFER_SIZE = 12000;
Accidential.
4) src/share/classes/sun/java2d/pisces/PiscesRenderingEngine.java
This is just adding white space ..
Sorry.
6) src/solaris/native/sun/awt/awt_GraphicsEnv.c
What was wrong with using the #define here ?
969 if (xerr-minor_code == X_ShmAttach) {
-
969 if (xerr-minor_code == 1) {
Gave me a compile-time problem on Rawhide, the IcedTea guys said hard
to fix, so I worked arround it.
- Clemens
[OpenJDK 2D-Dev] What is the offset-field used for in AATileGenerator.getAlpha?
Hi, What is the offset-field in AATileGenerator.getAlpha used for? The documentation/pseudo code in AATileGenerator seems to ignore this parameter. Does it mean I should rasterize moved by offset? Thanks in advance, Clemens
