[OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering
bourges.laurent at gmail.com
Mon Feb 8 20:58:35 UTC 2016
Jim & Phil,
I updated my previous webrev to have MarlinRenderingEngine TL/CLQ more
close to the new proposed approach in AAShapePipe (and looks simpler):
Please give me your comments on my previous email.
2016-02-06 0:21 GMT+01:00 Laurent Bourgès <bourges.laurent at gmail.com>:
> Jim & Phil,
> Here is an updated webrev according to jim's comments:
> - AAShapePipe rewritten to use a new ReentrantThreadLocal class
> (generalization) that use TL + CLQ (like Marlin) to use other (cached)
> TileState instances in case of reentrancy; such class may be useful for
> other use cases (replacing synchronized / static refs); it do not use
> WeakReference as TileState instances are very small (to be improved later ?)
> - Improved CrashPaintTest to check few pixel values to ensure rendering is
> PS: I noticed that AlphaPaintPipe has a constant int TILE_SIZE = 32 that
> seems useless as the method has a tilesize argument; maybe it should be
> cleaned up ?
> 2016-02-05 2:20 GMT+01:00 Jim Graham <james.graham at oracle.com>:
>> Ah, at least one error spotted here after my initial review...
>> The abox values could be changed by the code in lines 157-160 so you
>> can't cache their values until after that.
>> Also, as you pointed out in another thread in the grdev group, we should
>> probably do the same treatment for the TileState in case we have reentrance
>> for the AAShapePipe code...
>> On 2/4/2016 5:10 PM, Jim Graham wrote:
>>> Hi Laurent,
>>> In AAShapePipe you load the values from abox into variables named
>>> xywh, but these are not xywh values, they are min/max values. We
>>> typically use any of the following naming conventions for these types of
>>> - x0, y0, x1, y1
>>> - x1, y1, x2, y2
>>> - minX, minY, maxX, maxY
>>> Other than that naming inconsistency the changes look great...
>>> On 2/4/2016 2:21 PM, Laurent Bourgès wrote:
>>>> Please review the webrev fixing SEGFAULT (P2) in the Marlin Renderer
>>>> when using thread-local storage with custom Paint (reentrance):
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8148886
>>>> webrev: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8148886.0/
>>>> - detect reentrance in MarlinRenderingEngine using flag
>>>> RendererContext.usedTL (true/false) and use another context from CLQ
>>>> - added few more details in array checks (XXXArrayCache)
>>>> - fixed AAShapePipe to support reentrancy: added defensive copy of int
>>>> abox in local variables + TODO to discuss
>>>> - updated Version to 0.7.3.2
>>>> - fixed copyright headers
>>>> Best regards,
> Laurent Bourgès
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the 2d-dev