On Mon, 10 Nov 2025 13:59:30 GMT, John Hendrikx <[email protected]> wrote:
> This PR adds a `getDrawingContext` method to `WritableImage`, which works > similar to `Canvas::getGraphicsContext` and shares the same signatures. Key > features include: > > - **Shape rendering**: `strokeRect`, `fillRect`, `strokeOval`, `fillOval`, > `strokeArc`, `fillArc`, `strokePolyline`, `fillPolygon`, etc. > - **Stroke and fill attributes**: `lineWidth`, `lineCap`, `lineJoin`, > `miterLimit`, `fillRule`, `stroke` and `fill` paints. > - **Global graphics settings**: `globalAlpha` and `globalBlendMode`. > - **Image drawing**: draw other `Image` instances with scaling and > source/destination rectangles. > - **Text rendering**: render text > - **Save/Restore**: store current stroke, font, dashes, etc and restore them > later > - **Paths**: begin a path, with lines, curves, etc, then stroke or fill it > - **Clips**: support rectangular clips in the SW renderer > > This feature enables direct software rendering to `WritableImage` without > requiring a `Canvas` + snapshot. > > **Additional notes**: > > - The implementation leverages the software stack consisting of the Marlin > rasterizer and Pisces compositor/painter. > > **Example usage**: > > > WritableImage img = new WritableImage(400, 400); > DrawingContext ctx = img.getDrawingContext(); > ctx.setFill(Color.RED); > ctx.fillRect(50, 50, 100, 100); > > > See the sample program `RandomShapesDemo` to see a `WritableImage` and > `Canvas` side by side performing the same operations: > > <img width="1249" height="741" alt="image" > src="https://github.com/user-attachments/assets/4a0b9dcc-8f96-4faa-99cf-83c66d2c851e"; > /> > > Newer version: > > <img width="1249" height="741" alt="image" > src="https://github.com/user-attachments/assets/c0502620-3d02-4fbd-8c33-43bd5138b42c"; > /> > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). quite a few things. I would suggest adding unit tests for all the discovered problematic scenarios, if possible. modules/javafx.graphics/src/main/java/com/sun/prism/sw/SWDrawingContext.java line 201: > 199: public SWDrawingContext(com.sun.prism.Image img, Consumer<Rectangle> > pixelsDirty) { > 200: int[] data = switch (img.getPixelBuffer()) { > 201: case IntBuffer ib -> ib.array(); is it possible to get a sliced array at this point? IntBuffer::array() ignores the arrayOffset so the subsequent operations might overwrite wrong elements. modules/javafx.graphics/src/main/java/com/sun/prism/sw/SWDrawingContext.java line 258: > 256: @Override > 257: public void setGlobalAlpha(double alpha) { > 258: this.globalAlpha = Math.clamp(alpha, 0.0, 1.0); the clamping here violates the contract: "any valid double [value] can be set". the clamping should be done upon use. modules/javafx.graphics/src/main/java/com/sun/prism/sw/SWDrawingContext.java line 547: > 545: double y1 = Math.floor(Math.min(y, y + h)); > 546: double x2 = Math.ceil(Math.max(x, x + w)); > 547: double y2 = Math.ceil(Math.max(y, y + h)); the clip coordinates are rounded before transformation (L565), possibly repeatedly. later transformation will change the existing clip. should it be done at the last possible moment? modules/javafx.graphics/src/main/java/com/sun/prism/sw/SWDrawingContext.java line 987: > 985: applyStrokeParameters(); > 986: > 987: graphics.setTransform(BaseTransform.IDENTITY_TRANSFORM); will applying the `IDENTITY_TRANSFORM` prevent scaling of stroke attributes like line width and dash lengths? modules/javafx.graphics/src/main/java/com/sun/prism/sw/SWDrawingContext.java line 1153: > 1151: > 1152: if (stroke) { > 1153: graphics.draw(strike.getOutline(run, > BaseTransform.getTranslateInstance(runX, runY))); a wide stroke will go outside the min/max dirty bounds used later in L1167, possibly corrupting the pixels. modules/javafx.graphics/src/main/java/javafx/scene/canvas/GraphicsContext.java line 1409: > 1407: public void arcTo(double x1, double y1, double x2, double y2, > double radius) { > 1408: if (path.getNumCommands() == 0) { > 1409: moveTo(x1, y1); this needs lineTo(): this code will not render the expected dot using a round cap. modules/javafx.graphics/src/main/java/javafx/scene/canvas/GraphicsContext.java line 1413: > 1411: else { > 1412: try { > 1413: path.arcTo(curState.transform, (float) x1, (float) y1, > (float) x2, (float) y2, (float) radius); does it need `markPathDirty()` here? modules/javafx.graphics/src/main/java/javafx/scene/canvas/GraphicsContext.java line 1503: > 1501: @Override > 1502: public void clipRect(double x, double y, double w, double h) { > 1503: beginPath(); will this clobber the current path? scenario: build a path, call clipRect(), call fill() -> wrong path will be filled. modules/javafx.graphics/src/main/java/javafx/scene/image/WritableImage.java line 163: > 161: } > 162: > 163: private WeakReference<SWDrawingContext> drawingContextRef; is it right to use the weak reference here? what happens if the gc collects the current context, will the next call to getDrawingContext() return one with default paint, transform, etc.? modules/javafx.graphics/src/main/java/javafx/scene/image/WritableImage.java line 178: > 176: } > 177: > 178: context = new SWDrawingContext(img, rect -> > bufferDirty(rect)); a single `PixelBuffer` may source multiple `WriteableImage`s, this callback invalidates only that owned by the context, leaving the other siblings showing stale pixels. ------------- Changes requested by angorya (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/1969#pullrequestreview-5080469893 PR Review Comment: https://git.openjdk.org/jfx/pull/1969#discussion_r3906170191 PR Review Comment: https://git.openjdk.org/jfx/pull/1969#discussion_r3907141926 PR Review Comment: https://git.openjdk.org/jfx/pull/1969#discussion_r3907007408 PR Review Comment: https://git.openjdk.org/jfx/pull/1969#discussion_r3907028698 PR Review Comment: https://git.openjdk.org/jfx/pull/1969#discussion_r3907123699 PR Review Comment: https://git.openjdk.org/jfx/pull/1969#discussion_r3907160637 PR Review Comment: https://git.openjdk.org/jfx/pull/1969#discussion_r3906140684 PR Review Comment: https://git.openjdk.org/jfx/pull/1969#discussion_r3906979398 PR Review Comment: https://git.openjdk.org/jfx/pull/1969#discussion_r3906191680 PR Review Comment: https://git.openjdk.org/jfx/pull/1969#discussion_r3906957714
