On Mon, 6 Sep 2021 09:28:20 GMT, Jose Pereda <[email protected]> wrote:
>> modules/javafx.web/src/main/java/com/sun/webkit/WebPage.java line 97:
>>
>>> 95:
>>> 96: private int fontSmoothingType;
>>> 97: private Color backgroundColor = Color.WHITE;
>>
>> This might be a problem, since there are code paths that bypass
>> `setBackgroundColor(Color)`. I might recommend storing the converted 32-bit
>> color, and then checking that for transparency. Either that or you will need
>> to derive a `Color` from a 32-bit int in the cases that set a 32-bit int
>> color directly. The former is probably easier.
>
> In my first commit there was already a method to get the color out of the
> 32-bit int (which was still referred as hash value at that time):
>
> private static Color getColorFromHash(int hash) {
> String hexString = Integer.toHexString(hash);
> int length = hexString.length();
> return Color.valueOf("#" + "0".repeat(8 - length) + hexString);
> }
>
> If we keep it in `WebPage` (renaming it accordingly to `getColorFromInt32`
> for instance), we could do:
>
>
> public void setBackgroundColor(Color backgroundColor) {
> setBackgroundColor(getColorInt32Value(backgroundColor));
> }
>
> public void setBackgroundColor(int backgroundColor) {
> this.backgroundColor = getColorFromInt32(backgroundColor);
> lockPage();
> ...
> }
> ```
> which looks a little bit ugly.
>
> The other option, as you mention, is finding out if the 32-bit int has alpha
> 0 or 1, which can be done storing only the int value, not the color, so this
> looks cleaner, we don't really need to hold a reference to the Color after
> all:
>
>
> private int backgroundColor = -1; // Color.WHITE
>
> public void setBackgroundColor(Color backgroundColor) {
> setBackgroundColor(getColorInt32Value(backgroundColor));
> }
>
> public void setBackgroundColor(int backgroundColor) {
> this.backgroundColor = backgroundColor;
> lockPage();
> ...
> }
>
> private boolean isBackgroundTransparent() {
> return (backgroundColor & 0x000000FF) == 0;
> }
>
> private boolean isBackgroundOpaque() {
> return (backgroundColor & 0x000000FF) == 255;
> }
Yes, the second approach does look cleaner, although I think the initial
`backgroundColor` would be better defined as a hex constant, `0xffffffff` for
clarity.
-------------
PR: https://git.openjdk.java.net/jfx/pull/563
