Jim & Phil, Here is an updated webrev according to jim's comments: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8148886.1/
Changes: - 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 correct PS: I noticed that AlphaPaintPipe has a constant int TILE_SIZE = 32 that seems useless as the method has a tilesize argument; maybe it should be cleaned up ? Regards, Laurent 2016-02-05 2:20 GMT+01:00 Jim Graham <james.gra...@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... > > ...jim > > > On 2/4/2016 5:10 PM, Jim Graham wrote: > >> Hi Laurent, >> >> In AAShapePipe you load the values from abox[] into variables named >> xywh, but these are not xywh values, they are min/max values. We >> typically use any of the following naming conventions for these types of >> values: >> >> - x0, y0, x1, y1 >> - x1, y1, x2, y2 >> - minX, minY, maxX, maxY >> >> Other than that naming inconsistency the changes look great... >> >> ...jim >> >> 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/ >>> >>> Changes: >>> - 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 >>> >> -- -- Laurent Bourgès
