Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v13]
On Thu, 17 Sep 2020 06:35:16 GMT, Arun Joseph wrote: >> The fix works when the shape is displayed initially on the screen, but fails >> when we scroll the image off-screen and >> then, back. To see the issue, you need to either rotate the gradient >> transform (by 90 degrees) or use a circle (any >> shape other than a rectangle) as the mask shape, as this bug can't be seen >> using a mask rectangle. > > To reproduce the above issue, you can either modify ` id='Gradient' gradientTransform="rotate(90)">` or > ` > ` in `svgMask.html` and resize the > window for scrolling. This PR is on hold. It can be reopened or a new PR can be sent as and when it is ready to proceed. - PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v13]
On Thu, 17 Sep 2020 06:29:36 GMT, Arun Joseph wrote: >> Bhawesh Choudhary has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Updates as per review comments > > The fix works when the shape is displayed initially on the screen, but fails > when we scroll the image off-screen and > then, back. To see the issue, you need to either rotate the gradient > transform (by 90 degrees) or use a circle (any > shape other than a rectangle) as the mask shape, as this bug can't be seen > using a mask rectangle. To reproduce the above issue, you can either you `` or ` ` in `svgMask.html` and resize the window for scrolling. - PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v13]
On Mon, 14 Sep 2020 13:47:28 GMT, Bhawesh Choudhary wrote: >> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp >> in WebKit was not implemented, so masking >> doesn't take place at all while rendering SVGRect. to fix this issue add >> implementation of function clipToImageBuffer() >> in GraphicsContextJava.cpp and send clip image to >> WCGraphicsPrismContext.java While rendering in >> WCGraphicsPrismContext.java if image clip mask is available, use it for >> rendering using MaskTextureGraphics interface >> otherwise use usual way of rendering. > > Bhawesh Choudhary has updated the pull request incrementally with one > additional commit since the last revision: > > Updates as per review comments The fix works when the shape is displayed initially on the screen, but fails when we scroll the image off-screen and then, back. To see the issue, you need to either rotate the gradient transform (by 90 degrees) or use a circle (any shape other than a rectangle) as the mask shape, as this bug can't be seen using a mask rectangle. - PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v12]
On Fri, 11 Sep 2020 00:13:48 GMT, Kevin Rushforth wrote:
>> Bhawesh Choudhary has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Added unit test for strokes
>
> modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java
> line 524:
>
>> 522: }
>> 523: if (isFill) {
>> 524: g1.fill(shape);
>
> This will call the slower `fill(Shape)` method in all cases rather than the
> specialized `fillRect` or `fillRoundRect`
> method. Given all the other things that are done to draw a shape with a clip
> mask, I suspect that this is fine. One
> thing to consider is to pass in an enum instead of a boolean. The enum could
> say whether to use the specialized calls
> or the generic `fill` call. It's probably not worth the effort to make this
> change.
other than paths only RoundRectangle2D is passed to this function. Shape can be
checked if it is an instance of
RoundRectangle2D and faster draw api can be called. added the same along with
enum private to this class.
> modules/javafx.web/src/test/java/test/javafx/scene/web/SVGTest.java line 164:
>
>> 162: final WebPage webPage = WebEngineShim.getPage(getEngine());
>> 163: assertNotNull(webPage);
>> 164: final BufferedImage img = WebPageShim.paint(webPage, 0, 0,
>> 200, 200);
>
> This is added to the (preexisting) problem of calling `paint` on the wrong
> thread. In this case, it doesn't seem to
> cause any additional problems, and other tests in this same class do it, so
> we can fix this in the follow-on issue that
> is already filed,
> [JDK-8252596](https://bugs.openjdk.java.net/browse/JDK-8252596).
moved tests to system test. also consolidated all tests into one.
-
PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v12]
On Fri, 11 Sep 2020 00:10:29 GMT, Kevin Rushforth wrote:
>> Bhawesh Choudhary has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Added unit test for strokes
>
> modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java
> line 447:
>
>> 445: public void setClip(int cx, int cy, int cw, int ch, WCImage
>> maskImage) {
>> 446: setClip(new Rectangle(cx, cy, cw, ch));
>> 447: state.setClipMaskImage(maskImage);
>
> Should all of the other variants of setClip (the ones that don't take a
> maskImage) set the clipMaskImage to null?
> Otherwise it seems that it might not be reset to null in all cases.
added setting of null to maskImage for all the overloads of setClip where mask
image is not present.
> modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java
> line 520:
>
>> 518: state.apply(g1);
>> 519: g1.setPaint(paint);
>> 520: if(stroke != null) {
>
> Minor: there should be a space after the `if`.
Fixed
-
PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v7]
On Fri, 11 Sep 2020 06:48:35 GMT, Bhawesh Choudhary wrote: >> This question is still outstanding. It seems like the call to `setCTM` is >> either needed before all of the `setGradient` >> calls or none of them. Can you comment? > > i believe setCTM call is needed for none of them. it is a workaround i have > added till i have more concrete fix. also > please note that this workaround is needed only in cases where ui scaling is > more than 1. Removed the workaround and added right fix. `setCTM` call is not needed in any of the case. - PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v13]
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: Updates as per review comments - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/ed2dd092..f26f03df Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=213=12 - incr: https://webrevs.openjdk.java.net/?repo=jfx=213=11-12 Stats: 336 lines in 6 files changed: 173 ins; 152 del; 11 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v7]
On Fri, 11 Sep 2020 00:29:53 GMT, Kevin Rushforth wrote:
>> modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/GraphicsContextJava.cpp
>> line 235:
>>
>>> 233: } else {
>>> 234: if (m_state.fillGradient) {
>>> 235: setCTM(m_state.transform);
>>
>> Why is this needed here, but not in the other places `setGradient` is
>> called? Won't there be a similar problem with
>> `strokeRect`, `fillPath`, etc?
>
> This question is still outstanding. It seems like the call to `setCTM` is
> either needed before all of the `setGradient`
> calls or none of them. Can you comment?
i believe setCTM call is needed for none of them. it is a workaround i have
added till i have more concrete fix. also
please note that this workaround is needed only in cases where ui scaling is
more than 1.
-
PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v7]
On Fri, 7 Aug 2020 14:54:14 GMT, Kevin Rushforth wrote:
>> Bhawesh Choudhary has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Removed RenderSVGResourceMasker changes and added fix for HIDPI mask
>> rendering
>
> modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/GraphicsContextJava.cpp
> line 235:
>
>> 233: } else {
>> 234: if (m_state.fillGradient) {
>> 235: setCTM(m_state.transform);
>
> Why is this needed here, but not in the other places `setGradient` is called?
> Won't there be a similar problem with
> `strokeRect`, `fillPath`, etc?
This question is still outstanding. It seems like the call to `setCTM` is
either needed before all of the `setGradient`
calls or none of them. Can you comment?
-
PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v12]
On Tue, 8 Sep 2020 09:56:06 GMT, Bhawesh Choudhary
wrote:
>> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp
>> in WebKit was not implemented, so masking
>> doesn't take place at all while rendering SVGRect. to fix this issue add
>> implementation of function clipToImageBuffer()
>> in GraphicsContextJava.cpp and send clip image to
>> WCGraphicsPrismContext.java While rendering in
>> WCGraphicsPrismContext.java if image clip mask is available, use it for
>> rendering using MaskTextureGraphics interface
>> otherwise use usual way of rendering.
>
> Bhawesh Choudhary has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Added unit test for strokes
modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java
line 447:
> 445: public void setClip(int cx, int cy, int cw, int ch, WCImage
> maskImage) {
> 446: setClip(new Rectangle(cx, cy, cw, ch));
> 447: state.setClipMaskImage(maskImage);
Should all of the other variants of setClip (the ones that don't take a
maskImage) set the clipMaskImage to null?
Otherwise it seems that it might not be reset to null in all cases.
modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java
line 520:
> 518: state.apply(g1);
> 519: g1.setPaint(paint);
> 520: if(stroke != null) {
Minor: there should be a space after the `if`.
modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java
line 524:
> 522: }
> 523: if (isFill) {
> 524: g1.fill(shape);
This will call the slower `fill(Shape)` method in all cases rather than the
specialized `fillRect` or `fillRoundRect`
method. Given all the other things that are done to draw a shape with a clip
mask, I suspect that this is fine. One
thing to consider is to pass in an enum instead of a boolean. The enum could
say whether to use the specialized calls
or the generic `fill` call. It's probably not worth the effort to make this
change.
modules/javafx.web/src/test/java/test/javafx/scene/web/SVGTest.java line 164:
> 162: final WebPage webPage = WebEngineShim.getPage(getEngine());
> 163: assertNotNull(webPage);
> 164: final BufferedImage img = WebPageShim.paint(webPage, 0, 0,
> 200, 200);
This is added to the (preexisting) problem of calling `paint` on the wrong
thread. In this case, it doesn't seem to
cause any additional problems, and other tests in this same class do it, so we
can fix this in the follow-on issue that
is already filed,
[JDK-8252596](https://bugs.openjdk.java.net/browse/JDK-8252596).
-
PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v12]
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: Added unit test for strokes - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/ec272623..ed2dd092 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=213=11 - incr: https://webrevs.openjdk.java.net/?repo=jfx=213=10-11 Stats: 39 lines in 1 file changed: 37 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v11]
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: rendering fix for paths and strokes + refactoring - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/87f63074..ec272623 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=213=10 - incr: https://webrevs.openjdk.java.net/?repo=jfx=213=09-10 Stats: 132 lines in 1 file changed: 54 ins; 70 del; 8 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v10]
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: Added fillRoundedRect and fillPath drawing with mask - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/41f64c0e..87f63074 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=213=09 - incr: https://webrevs.openjdk.java.net/?repo=jfx=213=08-09 Stats: 42 lines in 1 file changed: 42 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v9]
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: Added unit tests - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/b1299ba1..41f64c0e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=213=08 - incr: https://webrevs.openjdk.java.net/?repo=jfx=213=07-08 Stats: 72 lines in 1 file changed: 71 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v8]
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision: - Merge branch 'master' into clip_mask_support - Removed RenderSVGResourceMasker changes and added fix for HIDPI mask rendering - HiDPI printing and Rendering fix - Removed unnecessery Ceil Functions - Refactoring, Utilize getFilterContext() function - Moved Printing drawing path to non MaskTextureGraphics interface - added dispose of resources - Formatting correction (Line Endings) - removed executable file mode - Added unit test + SW Graphics rendering part - ... and 2 more: https://git.openjdk.java.net/jfx/compare/31912a00...b1299ba1 - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/ef47709f..b1299ba1 Webrevs: - full: https://webrevs.openjdk.java.net/jfx/213/webrev.07 - incr: https://webrevs.openjdk.java.net/jfx/213/webrev.06-07 Stats: 404841 lines in 5801 files changed: 202048 ins; 139986 del; 62807 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v7]
On Tue, 28 Jul 2020 17:52:50 GMT, Bhawesh Choudhary
wrote:
>> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp
>> in WebKit was not implemented, so masking
>> doesn't take place at all while rendering SVGRect. to fix this issue add
>> implementation of function clipToImageBuffer()
>> in GraphicsContextJava.cpp and send clip image to
>> WCGraphicsPrismContext.java While rendering in
>> WCGraphicsPrismContext.java if image clip mask is available, use it for
>> rendering using MaskTextureGraphics interface
>> otherwise use usual way of rendering.
>
> Bhawesh Choudhary has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Removed RenderSVGResourceMasker changes and added fix for HIDPI mask
> rendering
While reviewing the most recent fix, I noticed that the call to `setCTM` in
`GraphicsContextJava.cpp` was only done in
the `fillRect` case. I then took a closer look at the change in
`WCGraphicsPrismContext.java` and I see that the
application of the mask is also only done for `fillRect`. A mask will still not
be applied for filled rounded
rectangles, filled paths, and all stroked primitives.
So this is an incomplete fix. I will add a couple additional test cases to the
bug report.
modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/GraphicsContextJava.cpp
line 235:
> 234: if (m_state.fillGradient) {
> 235: setCTM(m_state.transform);
> 236: setGradient(
Why is this needed here, but not in the other places `setGradient` is called?
Won't there be a similar problem with
`strokeRect`, `fillPath`, etc?
-
PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v7]
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: Removed RenderSVGResourceMasker changes and added fix for HIDPI mask rendering - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/312d068a..ef47709f Webrevs: - full: https://webrevs.openjdk.java.net/jfx/213/webrev.06 - incr: https://webrevs.openjdk.java.net/jfx/213/webrev.05-06 Stats: 3 lines in 2 files changed: 1 ins; 2 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v6]
On Fri, 24 Jul 2020 12:10:00 GMT, Arun Joseph wrote: >> Bhawesh Choudhary has updated the pull request incrementally with one >> additional commit since the last revision: >> >> HiDPI printing and Rendering fix > > modules/javafx.web/src/main/native/Source/WebCore/rendering/svg/RenderSVGResourceMasker.cpp > line 93: > >> 92: auto deviceScaleFactor = document().deviceScaleFactor(); >> 93: maskImageContext.applyDeviceScaleFactor(deviceScaleFactor); >> 94: > > For javafx port specific code in WebKit, it should be enclosed in `#if > PLATFORM(JAVA)`. But I think that there should > be a better approach as other ports don't require this scaling of > `deviceScaleFactor` RenderSVGResourceMasker.cpp changes are removed now. - PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v6]
On Tue, 21 Jul 2020 21:50:39 GMT, Bhawesh Choudhary wrote: >> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp >> in WebKit was not implemented, so masking >> doesn't take place at all while rendering SVGRect. to fix this issue add >> implementation of function clipToImageBuffer() >> in GraphicsContextJava.cpp and send clip image to >> WCGraphicsPrismContext.java While rendering in >> WCGraphicsPrismContext.java if image clip mask is available, use it for >> rendering using MaskTextureGraphics interface >> otherwise use usual way of rendering. > > Bhawesh Choudhary has updated the pull request incrementally with one > additional commit since the last revision: > > HiDPI printing and Rendering fix modules/javafx.web/src/main/native/Source/WebCore/rendering/svg/RenderSVGResourceMasker.cpp line 93: > 92: auto deviceScaleFactor = document().deviceScaleFactor(); > 93: maskImageContext.applyDeviceScaleFactor(deviceScaleFactor); > 94: For javafx port specific code in WebKit, it should be enclosed in `#if PLATFORM(JAVA)`. But I think that there should be a better approach as other ports don't require this scaling of `deviceScaleFactor` - PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v5]
On Fri, 12 Jun 2020 22:01:47 GMT, Kevin Rushforth wrote: >> The code looks good (with a couple minor formatting issues). >> >> All of the onscreen testing I did looks good on Windows. I'd like to test >> it on Mac as well. >> >> There is an issue with printing in the case of Hi-DPI scaling, which is what >> I run by default on Windows. The gradient >> texture appears to be scaled incorrectly (as if the scale was applied more >> than once). If I force scaling to 1 with >> `-Dglass.win.uiScale=1` then it prints correctly. > > It behaves the same on Mac with a Retina display as it does on Windows with > Hi-DPI scaling. The gradient doesn't appear > to be scaled correctly when printing. It's fine with on-screen rendering > (with both HW and SW pipeline). Issue related to hidpi rendering was caused due to correct pixel scale factor not being set to the context in which mask texture was getting rendered. setting correct device scale factor in `RenderSVGResourceMasker.cpp` fixed the issue. Below images shows the rendered mask texture in both case (HiDpi and Normal respectively)   Another issue was in Hi DPI printing. PrintGraphics draws with different resolution than the mask texture. Before the fix mask was not drawn correctly to RTTexture, due to which only top left portion which comes inside current draw bounds was taken to draw the whole image. After fix, entire mask texture is always considered while doing final drawing. - PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v6]
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: HiDPI printing and Rendering fix - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/f0c217ec..312d068a Webrevs: - full: https://webrevs.openjdk.java.net/jfx/213/webrev.05 - incr: https://webrevs.openjdk.java.net/jfx/213/webrev.04-05 Stats: 34 lines in 2 files changed: 5 ins; 9 del; 20 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: [Rev 04] RFR: 8218973: SVG with masking is not rendering image with mask effect
On Thu, 11 Jun 2020 20:47:12 GMT, Kevin Rushforth wrote: >> Bhawesh Choudhary has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Removed unnecessery Ceil Functions > > The code looks good (with a couple minor formatting issues). > > All of the onscreen testing I did looks good on Windows. I'd like to test it > on Mac as well. > > There is an issue with printing in the case of Hi-DPI scaling, which is what > I run by default on Windows. The gradient > texture appears to be scaled incorrectly (as if the scale was applied more > than once). If I force scaling to 1 with > `-Dglass.win.uiScale=1` then it prints correctly. It behaves the same on Mac with a Retina display as it does on Windows with Hi-DPI scaling. The gradient doesn't appear to be scaled correctly when printing. It's fine with on-screen rendering (with both HW and SW pipeline). - PR: https://git.openjdk.java.net/jfx/pull/213
Re: [Rev 04] RFR: 8218973: SVG with masking is not rendering image with mask effect
On Tue, 19 May 2020 10:13:44 GMT, Bhawesh Choudhary
wrote:
>> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp
>> in WebKit was not implemented, so masking
>> doesn't take place at all while rendering SVGRect. to fix this issue add
>> implementation of function clipToImageBuffer()
>> in GraphicsContextJava.cpp and send clip image to
>> WCGraphicsPrismContext.java While rendering in
>> WCGraphicsPrismContext.java if image clip mask is available, use it for
>> rendering using MaskTextureGraphics interface
>> otherwise use usual way of rendering.
>
> Bhawesh Choudhary has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Removed unnecessery Ceil Functions
The code looks good (with a couple minor formatting issues).
All of the onscreen testing I did looks good on Windows. I'd like to test it
on Mac as well.
There is an issue with printing in the case of Hi-DPI scaling, which is what I
run by default on Windows. The gradient
texture appears to be scaled incorrectly (as if the scale was applied more than
once). If I force scaling to 1 with
`-Dglass.win.uiScale=1` then it prints correctly.
modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java
line 531:
> 530: render(g, shadow, paint, null, node);
> 531: } else if(state.getClipMaskImageNoClone() != null) {
> 532: Rectangle rect = new Rectangle((int) x, (int) y,
> (int) w, (int) h);
space after `if`
modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java
line 553:
> 552: maskTexture.dispose();
> 553: if(g instanceof MaskTextureGraphics && !(g
> instanceof PrinterGraphics)) {
> 554: MaskTextureGraphics mg = (MaskTextureGraphics)
> (g);
space after `if`
-
PR: https://git.openjdk.java.net/jfx/pull/213
Re: [Rev 04] RFR: 8218973: SVG with masking is not rendering image with mask effect
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: Removed unnecessery Ceil Functions - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/a55f9f23..f0c217ec Webrevs: - full: https://webrevs.openjdk.java.net/jfx/213/webrev.04 - incr: https://webrevs.openjdk.java.net/jfx/213/webrev.03-04 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: [Rev 03] RFR: 8218973: SVG with masking is not rendering image with mask effect
On Mon, 18 May 2020 08:00:38 GMT, Bhawesh Choudhary wrote: >> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp >> in WebKit was not implemented, so masking >> doesn't take place at all while rendering SVGRect. to fix this issue add >> implementation of function clipToImageBuffer() >> in GraphicsContextJava.cpp and send clip image to >> WCGraphicsPrismContext.java While rendering in >> WCGraphicsPrismContext.java if image clip mask is available, use it for >> rendering using MaskTextureGraphics interface >> otherwise use usual way of rendering. > > Bhawesh Choudhary has updated the pull request incrementally with one > additional commit since the last revision: > > Refactoring, Utilize getFilterContext() function modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java line 536: > 535: RTTexture paintRtTexture = > g.getResourceFactory().createRTTexture( > 536: (int) Math.ceil(transformedRect.width), > 537: (int) Math.ceil(transformedRect.height), transformedRect's height and width are already of type int - PR: https://git.openjdk.java.net/jfx/pull/213
Re: [Rev 03] RFR: 8218973: SVG with masking is not rendering image with mask effect
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: Refactoring, Utilize getFilterContext() function - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/5b85b47d..a55f9f23 Webrevs: - full: https://webrevs.openjdk.java.net/jfx/213/webrev.03 - incr: https://webrevs.openjdk.java.net/jfx/213/webrev.02-03 Stats: 8 lines in 1 file changed: 0 ins; 7 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: [Rev 02] RFR: 8218973: SVG with masking is not rendering image with mask effect
On Sun, 10 May 2020 20:49:07 GMT, Bhawesh Choudhary
wrote:
>> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp
>> in WebKit was not implemented, so masking
>> doesn't take place at all while rendering SVGRect. to fix this issue add
>> implementation of function clipToImageBuffer()
>> in GraphicsContextJava.cpp and send clip image to
>> WCGraphicsPrismContext.java While rendering in
>> WCGraphicsPrismContext.java if image clip mask is available, use it for
>> rendering using MaskTextureGraphics interface
>> otherwise use usual way of rendering.
>
> Bhawesh Choudhary has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Moved Printing drawing path to non MaskTextureGraphics interface
modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java
line 560:
> 559: } else {
> 560: Screen screen = g.getAssociatedScreen();
> 561: FilterContext filterContext;
This logic is already present in getFilterContext(). You can call the function
instead.
-
PR: https://git.openjdk.java.net/jfx/pull/213
Re: [Rev 02] RFR: 8218973: SVG with masking is not rendering image with mask effect
On Fri, 8 May 2020 23:37:51 GMT, Kevin Rushforth wrote: >> Bhawesh Choudhary has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Moved Printing drawing path to non MaskTextureGraphics interface > > modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java > line 566: > >> 565: filterContext = >> PrFilterContext.getInstance(screen); >> 566: } >> 567: PrDrawable imagePrDrawable = >> PrDrawable.create(filterContext, paintRtTexture); > > Did you test both the SW pipeline and printing paths? Yes, used -Dprism.order=sw to run HelloWebView test to verify SW Pipeline Yes, i have added a modified HelloWebView test in attached bug which is used to test printing paths. https://bugs.openjdk.java.net/secure/attachment/88102/HelloWebView.java - PR: https://git.openjdk.java.net/jfx/pull/213
Re: [Rev 02] RFR: 8218973: SVG with masking is not rendering image with mask effect
On Fri, 8 May 2020 23:25:18 GMT, Kevin Rushforth wrote: >> Bhawesh Choudhary has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Moved Printing drawing path to non MaskTextureGraphics interface > > modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java > line 549: > >> 548: Texture.WrapMode.CLAMP_NOT_NEEDED); >> 549: RTTexture maskRtTexture = >> g.getResourceFactory().createRTTexture(nativeMaskImage.getWidth(), >> 550: nativeMaskImage.getHeight(), >> Texture.WrapMode.CLAMP_NOT_NEEDED); > > Why do you need to create a second RTT here? I would have thought you could > use the mask texture directly (you may need > to scale the image in order to do that). main problem with not using second RTTexture is the interface MaskTextureGraphics, which accept RTTexture only. also it difficult to add new API taking Texture instead of RTTexture given that SW Pipeline and J2D pipeline has direct dependency on RTTexture interface. - PR: https://git.openjdk.java.net/jfx/pull/213
Re: [Rev 02] RFR: 8218973: SVG with masking is not rendering image with mask effect
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: Moved Printing drawing path to non MaskTextureGraphics interface - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/e66fa3bc..5b85b47d Webrevs: - full: https://webrevs.openjdk.java.net/jfx/213/webrev.02 - incr: https://webrevs.openjdk.java.net/jfx/213/webrev.01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: [Rev 01] RFR: 8218973: SVG with masking is not rendering image with mask effect
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. Bhawesh Choudhary has updated the pull request incrementally with one additional commit since the last revision: added dispose of resources - Changes: - all: https://git.openjdk.java.net/jfx/pull/213/files - new: https://git.openjdk.java.net/jfx/pull/213/files/c2729a9c..e66fa3bc Webrevs: - full: https://webrevs.openjdk.java.net/jfx/213/webrev.01 - incr: https://webrevs.openjdk.java.net/jfx/213/webrev.00-01 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
Re: RFR: 8218973: SVG with masking is not rendering image with mask effect
On Thu, 7 May 2020 09:19:28 GMT, Bhawesh Choudhary wrote: > Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp > in WebKit was not implemented, so masking > doesn't take place at all while rendering SVGRect. to fix this issue add > implementation of function clipToImageBuffer() > in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java > While rendering in > WCGraphicsPrismContext.java if image clip mask is available, use it for > rendering using MaskTextureGraphics interface > otherwise use usual way of rendering. I verified that this does fix the bug, although there is a resource issue you will need to address. When I run a simple test (the one attached to the bug report) I get the following warning messages: Outstanding resource locks detected: D3D Vram Pool: 21,073,412 used (3.9%), 67,108,864 target (12.5%), 536,870,912 max 13 total resources being managed average resource age is 0.6 frames 0 resources at maximum supported age (0.0%) 5 resources marked permanent (38.5%) 3 resources have had mismatched locks (23.1%) 3 resources locked (23.1%) 8 resources contain interesting data (61.5%) 0 resources disappeared (0.0%) You need to `unlock()` and `dispose()` the temporary RTTs when you are done with them. You should also `dispose()` the temporary Texture. I left a couple inline comments as well, the main question being about whether we really need a second RTT for the mask texture. modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java line 549: > 548: Texture.WrapMode.CLAMP_NOT_NEEDED); > 549: RTTexture maskRtTexture = > g.getResourceFactory().createRTTexture(nativeMaskImage.getWidth(), > 550: nativeMaskImage.getHeight(), > Texture.WrapMode.CLAMP_NOT_NEEDED); Why do you need to create a second RTT here? I would have thought you could use the mask texture directly (you may need to scale the image in order to do that). modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java line 566: > 565: filterContext = > PrFilterContext.getInstance(screen); > 566: } > 567: PrDrawable imagePrDrawable = > PrDrawable.create(filterContext, paintRtTexture); Did you test both the SW pipeline and printing paths? - PR: https://git.openjdk.java.net/jfx/pull/213
RFR: 8218973: SVG with masking is not rendering image with mask effect
Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp in WebKit was not implemented, so masking doesn't take place at all while rendering SVGRect. to fix this issue add implementation of function clipToImageBuffer() in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java While rendering in WCGraphicsPrismContext.java if image clip mask is available, use it for rendering using MaskTextureGraphics interface otherwise use usual way of rendering. - Commit messages: - Formatting correction (Line Endings) - removed executable file mode - Added unit test + SW Graphics rendering part - Pixel scale issue fix - 8218973: SVG with masking is not rendering image with mask effect Changes: https://git.openjdk.java.net/jfx/pull/213/files Webrev: https://webrevs.openjdk.java.net/jfx/213/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8218973 Stats: 132 lines in 6 files changed: 130 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/213.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213 PR: https://git.openjdk.java.net/jfx/pull/213
