Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v9]
On Fri, 10 Sep 2021 18:15:21 GMT, Jose Pereda wrote: >> Currently, `WebPage` has already a public `setBackgroundColor()` method, but >> the class is not public. Therefore, public API is needed in `WebView` to >> allow developers access to it. >> >> In line with the `fontSmoothingType` property, this PR provides public >> support for setting the background color of a WebPage, by adding a >> `pageFill` property, and a CSR is required. >> >> The color for the background, that can be opaque, transparent or with any >> level of opacity, can be set via code or via CSS using `-fx-page-fill`. >> >> Unit tests and a system test are provided. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Fix test to pass all 3 platforms Great finding, thanks for clarifying it. I've tested it successfully too. I'll commit it then. - PR: https://git.openjdk.java.net/jfx/pull/563
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v9]
On Fri, 10 Sep 2021 18:15:21 GMT, Jose Pereda wrote: >> Currently, `WebPage` has already a public `setBackgroundColor()` method, but >> the class is not public. Therefore, public API is needed in `WebView` to >> allow developers access to it. >> >> In line with the `fontSmoothingType` property, this PR provides public >> support for setting the background color of a WebPage, by adding a >> `pageFill` property, and a CSR is required. >> >> The color for the background, that can be opaque, transparent or with any >> level of opacity, can be set via code or via CSS using `-fx-page-fill`. >> >> Unit tests and a system test are provided. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Fix test to pass all 3 platforms I verified that the above change works, and doesn't cause any regressions that I can see. - PR: https://git.openjdk.java.net/jfx/pull/563
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v9]
On Fri, 10 Sep 2021 18:15:21 GMT, Jose Pereda wrote: >> Currently, `WebPage` has already a public `setBackgroundColor()` method, but >> the class is not public. Therefore, public API is needed in `WebView` to >> allow developers access to it. >> >> In line with the `fontSmoothingType` property, this PR provides public >> support for setting the background color of a WebPage, by adding a >> `pageFill` property, and a CSR is required. >> >> The color for the background, that can be opaque, transparent or with any >> level of opacity, can be set via code or via CSS using `-fx-page-fill`. >> >> Unit tests and a system test are provided. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Fix test to pass all 3 platforms > Somehow by accident I've found out that doing in `WebPage::paint2GC`: > > ``` > +gc.clearRect(0, 0, 0, 0); // extra call to clear rect > ``` > > fixes the issue for the Group test (same for no container case). Interesting. > After some testing, I modified this method to get it working with the > expected single call with just this change: > > ``` > -// set the blend mode to CLEAR > -context.updateCompositeMode(CompositeMode.CLEAR); > Paint oldPaint = getPaint(); > setPaint(Color.BLACK); // any color will do... > fillQuad(x1, y1, x2, y2); > +// set the blend mode to CLEAR > +context.updateCompositeMode(CompositeMode.CLEAR); > context.flushVertexBuffer(); > ``` > > which seems to indicate that `fillQuad` requires SRC_OVER and we can use the > original black color, and only before flushing we set the CLEAR mode. Even more interesting. > Does this make sense? No, it doesn't make sense, since setting the mode to CLEAR before drawing the quad is the right order. The fact that it matters indicates that something very odd is happening with state management. Your experiment was the clue that allowed me to figure out the problem. The problem is that the call to `context.updateCompositeMode` is simply the wrong thing to do. The right call, which is what the D3D pipeline is doing, is `setCompositeMode`. The former does not update the state so any other method that validates the state will use the wrong composite mode. It's amazing to me that this went undetected for this long, and that it hasn't caused other problems. Here is the proposed change: CompositeMode mode = getCompositeMode(); // set the blend mode to CLEAR -context.updateCompositeMode(CompositeMode.CLEAR); +setCompositeMode(CompositeMode.CLEAR); Paint oldPaint = getPaint(); setPaint(Color.BLACK); // any color will do... fillQuad(x1, y1, x2, y2); context.flushVertexBuffer(); setPaint(oldPaint); // restore default blend mode -context.updateCompositeMode(mode); +setCompositeMode(mode); This will need to be well tested on Linux and Mac (a full set of Robot tests). - PR: https://git.openjdk.java.net/jfx/pull/563
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v9]
On Fri, 10 Sep 2021 18:15:21 GMT, Jose Pereda wrote:
>> Currently, `WebPage` has already a public `setBackgroundColor()` method, but
>> the class is not public. Therefore, public API is needed in `WebView` to
>> allow developers access to it.
>>
>> In line with the `fontSmoothingType` property, this PR provides public
>> support for setting the background color of a WebPage, by adding a
>> `pageFill` property, and a CSR is required.
>>
>> The color for the background, that can be opaque, transparent or with any
>> level of opacity, can be set via code or via CSS using `-fx-page-fill`.
>>
>> Unit tests and a system test are provided.
>
> Jose Pereda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix test to pass all 3 platforms
Somehow by accident I've found out that doing in `WebPage::paint2GC`:
if (clip != null) {
if (isBackgroundColorTransparent()) {
+gc.clearRect(0, 0, 0, 0); // extra call to clear rect
gc.clearRect((int) clip.getX(), (int) clip.getY(),
(int) clip.getWidth(), (int) clip.getHeight());
}
gc.setClip(clip);
fixes the issue for the Group test (same for no container case).
Ultimately, this is the same as calling twice `ES2Graphics::clearQuad`.
After some testing, I modified this method to get it working with the expected
single call with just this change:
CompositeMode mode = getCompositeMode();
-// set the blend mode to CLEAR
-context.updateCompositeMode(CompositeMode.CLEAR);
Paint oldPaint = getPaint();
setPaint(Color.BLACK); // any color will do...
fillQuad(x1, y1, x2, y2);
+// set the blend mode to CLEAR
+context.updateCompositeMode(CompositeMode.CLEAR);
context.flushVertexBuffer();
which seems to indicate that `fillQuad` requires SRC_OVER and we can use the
original black color, and only before flushing we set the CLEAR mode.
Does this make sense?
-
PR: https://git.openjdk.java.net/jfx/pull/563
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v9]
On Fri, 10 Sep 2021 18:15:21 GMT, Jose Pereda wrote:
>> Currently, `WebPage` has already a public `setBackgroundColor()` method, but
>> the class is not public. Therefore, public API is needed in `WebView` to
>> allow developers access to it.
>>
>> In line with the `fontSmoothingType` property, this PR provides public
>> support for setting the background color of a WebPage, by adding a
>> `pageFill` property, and a CSR is required.
>>
>> The color for the background, that can be opaque, transparent or with any
>> level of opacity, can be set via code or via CSS using `-fx-page-fill`.
>>
>> Unit tests and a system test are provided.
>
> Jose Pereda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix test to pass all 3 platforms
Thanks for testing it, I can reproduce the issue indeed.
I had my interactive test too which doesn't fail with alpha = 0, and comparing
both tests I realise now that in my case I was using a transparent VBox instead
of a Group as a container for the WebView, and later on when I created the
system test, I removed such container.
If you try:
webView.setPageFill(Color.TRANSPARENT);
var root = new VBox();
root.setStyle("-fx-background-color: transparent");
root.getChildren().add(webView);
that should work for you as well...
Obviously there is an issue with alpha = 0 in case we use Group or not
container at all, vs when we use a Region.
The comment at
[Region::doComputeGeomBounds](https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java#L3310)
my be relevant though?
> the geom bounds forms the basis of the dirty regions
-
PR: https://git.openjdk.java.net/jfx/pull/563
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v9]
On Fri, 10 Sep 2021 18:15:21 GMT, Jose Pereda wrote: >> Currently, `WebPage` has already a public `setBackgroundColor()` method, but >> the class is not public. Therefore, public API is needed in `WebView` to >> allow developers access to it. >> >> In line with the `fontSmoothingType` property, this PR provides public >> support for setting the background color of a WebPage, by adding a >> `pageFill` property, and a CSR is required. >> >> The color for the background, that can be opaque, transparent or with any >> level of opacity, can be set via code or via CSS using `-fx-page-fill`. >> >> Unit tests and a system test are provided. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Fix test to pass all 3 platforms The test now passes on one of my Linux machines, but I still see the smearing of the text when scrolling. I took the html file from your test and used it in a simple interactive program using a transparent background that will show the problem. If you run the test program on either Mac or Linux (using the es2 pipeline) and scroll the window, you will see that the scroll area isn't cleared. I found that if the color has any alpha > 0 it works correctly. This means that something is behaving differently with fully transparent (alpha == 0) in the ES2 pipeline. It might be related to why you needed to change the initial fill color for the ES2 pipeline. [SimpleWebViewTransp.java.txt](https://github.com/openjdk/jfx/files/7148102/SimpleWebViewTransp.java.txt) - PR: https://git.openjdk.java.net/jfx/pull/563
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v9]
On Fri, 10 Sep 2021 18:15:21 GMT, Jose Pereda wrote: >> Currently, `WebPage` has already a public `setBackgroundColor()` method, but >> the class is not public. Therefore, public API is needed in `WebView` to >> allow developers access to it. >> >> In line with the `fontSmoothingType` property, this PR provides public >> support for setting the background color of a WebPage, by adding a >> `pageFill` property, and a CSR is required. >> >> The color for the background, that can be opaque, transparent or with any >> level of opacity, can be set via code or via CSS using `-fx-page-fill`. >> >> Unit tests and a system test are provided. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Fix test to pass all 3 platforms I'll test on all three platforms and report back. - PR: https://git.openjdk.java.net/jfx/pull/563
Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v9]
> Currently, `WebPage` has already a public `setBackgroundColor()` method, but > the class is not public. Therefore, public API is needed in `WebView` to > allow developers access to it. > > In line with the `fontSmoothingType` property, this PR provides public > support for setting the background color of a WebPage, by adding a `pageFill` > property, and a CSR is required. > > The color for the background, that can be opaque, transparent or with any > level of opacity, can be set via code or via CSS using `-fx-page-fill`. > > Unit tests and a system test are provided. Jose Pereda has updated the pull request incrementally with one additional commit since the last revision: Fix test to pass all 3 platforms - Changes: - all: https://git.openjdk.java.net/jfx/pull/563/files - new: https://git.openjdk.java.net/jfx/pull/563/files/90ab86d4..40302d8c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=563=08 - incr: https://webrevs.openjdk.java.net/?repo=jfx=563=07-08 Stats: 17 lines in 1 file changed: 2 ins; 0 del; 15 mod Patch: https://git.openjdk.java.net/jfx/pull/563.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/563/head:pull/563 PR: https://git.openjdk.java.net/jfx/pull/563
