Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v10]
> 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: Use correct call to set composite mode when clearing quads and restore color - Changes: - all: https://git.openjdk.java.net/jfx/pull/563/files - new: https://git.openjdk.java.net/jfx/pull/563/files/40302d8c..b5088556 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=563=09 - incr: https://webrevs.openjdk.java.net/?repo=jfx=563=08-09 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 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
Re: RFR: 8271398: GTK3 drag view image swaps red and blue color channels [v4]
On Fri, 20 Aug 2021 22:22:51 GMT, Thiago Milczarek Sayao
wrote:
>> It seems raw images need to be converted BRGA -> RGBA.
>>
>> It was being converted on gtk2 code path, but gtk3 only uses
>> `gtk_drag_set_icon_pixbuf`.
>>
>> I have simplified the gtk2 `DragView::View::expose` to paint with
>> `gdk_cairo_set_source_pixbuf` (that is available since Gtk 2.8) because the
>> old way was somehow converting again.
>>
>> Run the issue sample with `-Djdk.gtk.version=2` to test the gtk2 code path.
>>
>> To test:
>>
>> `./gradlew apps`
>>
>>
>> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar
>> dragdrop.DragDropWithControls
>> java @build/run.args -cp apps/toys/DragDrop/dist/DragDrop.jar
>> dragdrop.DragDropColor
>>
>> java -Djdk.gtk.version=2 @build/run.args -cp
>> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropWithControls
>> java -Djdk.gtk.version=2 @build/run.args -cp
>> apps/toys/DragDrop/dist/DragDrop.jar dragdrop.DragDropColor
>
> Thiago Milczarek Sayao has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Review requests.
This fix introduces a memory leak, but otherwise looks correct. I verified that
with gtk3 the new manual test passes with the fix and fails without the fix
(with gtk2 is passes before and after the fix).
modules/javafx.graphics/src/main/native-glass/gtk/glass_dnd.cpp line 916:
> 914:
> 915: if (is_raw_image) {
> 916: data = (guchar*) convert_BGRA_to_RGBA((const
> int*) data, w * 4, h);
This will leak memory, since the original buffer allocated by the call to
`g_try_malloc0` on line 911 is never freed. I recommend something like this:
guchar* origdata = data;
data = (guchar*) convert_BGRA_to_RGBA((const int*) origdata, w * 4, h);
g_free(origdata);
-
PR: https://git.openjdk.java.net/jfx/pull/599
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: 8272870: Add convenience factory methods for border and background [v3]
On Thu, 16 Sep 2021 21:15:20 GMT, Kevin Rushforth wrote: > I agree that it needs consensus, so the question is how many apps would use > it from code (as opposed to CSS), and be satisfied with the other defaults. I > don't object to adding a 2nd variant that takes a stroke width as long as we > stop there. I don't want variants that take style, or corner radii, or > insets, etc. (at that point, just use the existing API). My thoughts as well, though if people care about all sorts of combinations, a builder pattern would be the way in my opinion. - PR: https://git.openjdk.java.net/jfx/pull/610
Re: RFR: 8272870: Add convenience factory methods for border and background [v3]
On Thu, 9 Sep 2021 23:43:29 GMT, Nir Lisker wrote:
>> Added convenience factory factory methods for Background and Border.
>
> Nir Lisker has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Removed whitespaces
> - Added tests and doc updates
> > ```
> > public static Border stroke(Paint stroke, double width) {
> > ```
> > ...
> > But I really want to hear other opinions. This can also be a follow up. :)
>
> I don't mind adding this variant, but it needs consensus.
I agree that it needs consensus, so the question is how many apps would use it
from code (as opposed to CSS), and be satisfied with the other defaults. I
don't object to adding a 2nd variant that takes a stroke width as long as we
stop there. I don't want variants that take style, or corner radii, or insets,
etc. (at that point, just use the existing API).
-
PR: https://git.openjdk.java.net/jfx/pull/610
Re: RFR: 8272870: Add convenience factory methods for border and background [v3]
On Thu, 16 Sep 2021 08:36:49 GMT, Marius Hanl wrote:
> One comment from my side: I would find it quite useful if we have another
> border factory method with a double as the second parameter which let us
> specify the border width. So e.g.
>
> ```
> public static Border stroke(Paint stroke, double width) {
> return new Border(new BorderStroke(stroke, BorderStrokeStyle.SOLID, null,
> new BorderWidths(width)));
> }
> ```
>
> But I really want to hear other opinions. This can also be a follow up. :)
I don't mind adding this variant, but it needs consensus.
-
PR: https://git.openjdk.java.net/jfx/pull/610
Re: RFR: 8172095: Let Node.managed become CSS-styleable [v2]
On Sat, 4 Sep 2021 15:44:17 GMT, Abhinay Agarwal wrote: >> 8172095: Let Node.managed become CSS-styleable > > Abhinay Agarwal has updated the pull request incrementally with two > additional commits since the last revision: > > - Add default value tests for setting / getting the managed property > - Replace with HTML equivalent code Looks good. This is ready to be integrated once the CSR is approved. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/602
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: 8255015: Inconsistent illumination of 3D shape by PointLight [v3]
> The inconsistent illumination happens on Macs with retina displays only if > the 3D shape is placed in a SubScene. The light sources are located with > wrong coordinates in sub scenes and this causes a different illumination. The > wrong coordinates for the light sources come from the fact that the retina > pixel scale factors are not used in a SubScene. > > With this pull request, the retina pixel scale factors will be also used in > SubScenes and this should resolve the bug > [https://bugs.openjdk.java.net/browse/JDK-8255015](url) Andreas Heger 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 three additional commits since the last revision: - Merge branch 'openjdk:master' into fix-8255015 - Merge branch 'openjdk:master' into fix-8255015 - 8255015: Copy pixel scale factors from graphics object to subscene graphics so that the position of lights will be scaled correctly on retina displays - Changes: - all: https://git.openjdk.java.net/jfx/pull/531/files - new: https://git.openjdk.java.net/jfx/pull/531/files/3973a2e2..fdb1ddbe Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=531=02 - incr: https://webrevs.openjdk.java.net/?repo=jfx=531=01-02 Stats: 5975 lines in 27 files changed: 5975 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/531.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/531/head:pull/531 PR: https://git.openjdk.java.net/jfx/pull/531
Re: RFR: 8273485: Deadlock when also using Swing and exiting Fullscreen on Mac [v4]
> When using Swing it's possible to generate a Deadlock. > It's related to the nested eventloop started in enterFullScreenExitingLoop - > and the RenderLock aquired when using setView in Scene. > Sample Programm and Threaddump are added to the ticket. > > Removing the nested loop fixes the Problem. > I hope this doesn't have any side effect - so far i don't know of any. Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision: JDK-8273485 small cleanup of the changes. - Changes: - all: https://git.openjdk.java.net/jfx/pull/622/files - new: https://git.openjdk.java.net/jfx/pull/622/files/417ac2fe..8bee7a41 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=622=03 - incr: https://webrevs.openjdk.java.net/?repo=jfx=622=02-03 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/622.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/622/head:pull/622 PR: https://git.openjdk.java.net/jfx/pull/622
[jfx11u] Integrated: 8273732: Clarify review policies for clean backports in JavaFX update releases
On Thu, 16 Sep 2021 12:27:00 GMT, Kevin Rushforth wrote: > Clean backport of update to `CONTRIBUTING.md` to clarify that clean backports > of approved fixes can be integrated without further review. This pull request has now been integrated. Changeset: 896089eb Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx11u/commit/896089ebe7764d0b9503c394b5d4ed2a639e9b4a Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod 8273732: Clarify review policies for clean backports in JavaFX update releases Backport-of: 549bfef95e4c2e690eb83523a3bf5a8344c072f6 - PR: https://git.openjdk.java.net/jfx11u/pull/53
[jfx11u] Integrated: 8273732: Clarify review policies for clean backports in JavaFX update releases
Clean backport of update to `CONTRIBUTING.md` to clarify that clean backports of approved fixes can be integrated without further review. - Commit messages: - 8273732: Clarify review policies for clean backports in JavaFX update releases Changes: https://git.openjdk.java.net/jfx11u/pull/53/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx11u=53=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273732 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx11u/pull/53.diff Fetch: git fetch https://git.openjdk.java.net/jfx11u pull/53/head:pull/53 PR: https://git.openjdk.java.net/jfx11u/pull/53
[jfx17u] Integrated: 8273732: Clarify review policies for clean backports in JavaFX update releases
On Wed, 15 Sep 2021 22:05:26 GMT, Kevin Rushforth wrote: > Added a paragraph indicating that a review of a clean backport to an update > release is optional, if the bug in question has been approved for inclusion > into the release. This pull request has now been integrated. Changeset: 549bfef9 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx17u/commit/549bfef95e4c2e690eb83523a3bf5a8344c072f6 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod 8273732: Clarify review policies for clean backports in JavaFX update releases Reviewed-by: prr, pbansal, jvos - PR: https://git.openjdk.java.net/jfx17u/pull/9
Re: RFR: 8272870: Add convenience factory methods for border and background [v3]
On Thu, 9 Sep 2021 23:43:29 GMT, Nir Lisker wrote:
>> Added convenience factory factory methods for Background and Border.
>
> Nir Lisker has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Removed whitespaces
> - Added tests and doc updates
One comment from my side: I would find it quite useful if we have another
border factory method with a double as the second parameter which let us
specify the border width. So e.g.
public static Border stroke(Paint stroke, double width) {
return new Border(new BorderStroke(stroke, BorderStrokeStyle.SOLID, null,
new BorderWidths(width)));
}
But I really want to hear other opinions. This can also be a follow up. :)
-
PR: https://git.openjdk.java.net/jfx/pull/610
Re: [jfx17u] RFR: 8273732: Clarify review policies for clean backports in JavaFX update releases [v2]
On Wed, 15 Sep 2021 23:06:39 GMT, Kevin Rushforth wrote: >> Added a paragraph indicating that a review of a clean backport to an update >> release is optional, if the bug in question has been approved for inclusion >> into the release. > > Kevin Rushforth has updated the pull request incrementally with one > additional commit since the last revision: > > Minor wording update This clarifies it indeed. - Marked as reviewed by jvos (Reviewer). PR: https://git.openjdk.java.net/jfx17u/pull/9
