RFR: 8090547: Allow for transparent backgrounds in WebView [v4]

[Kevin Rushforth]

On Fri, 27 Aug 2021 09:32:24 GMT, Jose Pereda <jpereda at openjdk.org> wrote:

>> modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCGraphicsPrismContext.java line 459:
>> 
>>> 457:     public void setClip(WCRectangle c) {
>>> 458:         if (!isOpaque) {
>>> 459:             clearRect((int)c.getX(), (int)c.getY(),
>> 
>> Are you sure that there are no ill effects from clearing the rectangle every time a clip is set on a non-opaque context? This seems like a surprising side effect.
>
> Initially, this was needed when there was some level of transparency: when scrolling the old content was not cleared and you could see it at its old position.
> 
> For the full transparency case, this is still the case, but for translucent colors (0 < opacity < 1) I can't reproduce it anymore, so I'll modify this to apply only `if (isTransparent) { clearRect(); }`.
> 
> I don't see a performance drop because of this.

I'm more worried about correctness than performance. Setting a clip does not necessarily imply that the clipped region should be cleared. So this feels a bit like a workaround for a missing `clearRect` elsewhere. I wonder if there is code somewhere that assumes that if the fill color is fully transparent it can skip the call to `clearRect`?

>> modules/javafx.web/src/main/java/javafx/scene/web/WebView.java line 733:
>> 
>>> 731:                     Color color = get();
>>> 732:                     page.setBackgroundColor(color != null ? color.hashCode() :
>>> 733:                             DEFAULT_PAGE_FILL.hashCode());
>> 
>> This is relying on an undocumented, implementation detail. The current implementation of `Color::hashCode` happens to do what you want, but it is not specified. It seems safer to create a utility method to do this.
>
> Yes, that makes total sense. 
> 
> In fact, there was also the need to add a new [method](https://github.com/openjdk/jfx/pull/563/files#diff-b80bc720bf639cde38c5197a7619561221abcd34fb9ff7a933f4b932a1f36735R2579) in `WebPage` to read back the color from the int value, so I was thinking that it would be better to add a new method to `WebPage` like:
> 
> 
> public void setBackgroundColor(Color backgroundColor) {
>      int int32Color = WebPage.getBackgroundInt32Color(backgroundColor);
>      setBackgroundColor(int32Color);
> }
> 
> private static int getBackgroundInt32Color(Color color) {
> // implementation similar to Color::hashCode
> }
> 
> and from webView we could simply do:
> 
> page.setBackgroundColor(color);
> 
> 
> Thoughts?

Yes, this seems a good solution.

-------------

PR: https://git.openjdk.java.net/jfx/pull/563