Re: RFR: 8211294: [windows] TextArea content is blurry with 125% scaling
On Tue, 15 Dec 2020 02:02:37 GMT, Kevin Rushforth wrote:
>>>
>>>
>>> One more comment: given the quality problems that necessarily arise when
>>> the translation of a cached image is not on an integer boundary, part of
>>> the solution might be to snap the cached image to a pixel boundary as is
>>> done in this PR, but we would need to ensure that this doesn't impact
>>> smooth scrolling of a TextArea.
>>
>> I've done some quick tests with the minimal sample below, and could not
>> notice any performance impact with this patch compared to 15.0.1.
>>
>> java
>> public class TextScroll extends Application {
>>
>> @Override
>> public void start(Stage stage) throws Exception {
>> var textArea = new TextArea();
>> var txt = new
>> File(getClass().getResource("/Scrollpane.java").getFile());
>> textArea.setText(Files.readString(txt.toPath(),
>> StandardCharsets.UTF_8));
>> var root = new ScrollPane(textArea);
>> root.setFitToHeight(true);
>> root.setFitToWidth(true);
>> Scene scene;
>> scene = new Scene(root, 800, 600);
>> stage.setTitle("Scroll test");
>> stage.setScene(scene);
>> stage.show();
>> }
>>
>> public static void main(String[] args) {
>> launch(args);
>> }
>> }
>
> Actually, when I mentioned smooth scrolling, it wasn't performance I was
> thinking of, it was the ability to scroll by fractional pixel amounts, but
> with the default snap-to-pixel enabled, that won't happen anyway.
>
> I'll look into the other questions you raised tomorrow, specifically what we
> might want to do about rendering the cache, and also your question about
> snapToPixel.
>
> As for the specific problem with ScrollPane, I found the cause of that one.
> There is a (long-standing from the look of it) bug in
> [`Region::updateSnappedInsets`](https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java#L990).
> It attempts to account for snap-to-pixel, but does so incorrectly, using
> `Math.ceil` without taking screen scaling into account (that is, without
> calling the `snapXxxx` methods). The correct code should be something like:
>
> final boolean snap = isSnapToPixel();
> snappedTopInset = snapSizeY(insets.getTop(), snap);
> snappedRightInset = snapSizeX(insets.getRight(), snap);
> snappedBottomInset = snapSizeY(insets.getBottom(), snap);
> snappedLeftInset = snapSizeX(insets.getLeft(), snap);
>
> This fixes the ScrollPane problem without needing to modify the cached
> rendering code.
Thanks for that.
I am wondering; is it ok to potentially address both sub-issues discussed here
(Scrollpane snaping vs cache rendering) under the same JBS bug? Or would it be
better to address them under separate issues?
Speaking of which, the JBS issue title is in fact misleading, as the issue
appears to be not specific to Windows; what's the best practice in such cases;
rename the issue? Open a new one? Leave it as is?
-
PR: https://git.openjdk.java.net/jfx/pull/308
Re: RFR: 8211294: [windows] TextArea content is blurry with 125% scaling
On Mon, 14 Dec 2020 12:06:37 GMT, Frederic Thevenet
wrote:
>> One more comment: given the quality problems that necessarily arise when the
>> translation of a cached image is not on an integer boundary, part of the
>> solution might be to snap the cached image to a pixel boundary as is done in
>> this PR, but we would need to ensure that this doesn't impact smooth
>> scrolling of a TextArea.
>
>>
>>
>> One more comment: given the quality problems that necessarily arise when the
>> translation of a cached image is not on an integer boundary, part of the
>> solution might be to snap the cached image to a pixel boundary as is done in
>> this PR, but we would need to ensure that this doesn't impact smooth
>> scrolling of a TextArea.
>
> I've done some quick tests with the minimal sample below, and could not
> notice any performance impact with this patch compared to 15.0.1.
>
> java
> public class TextScroll extends Application {
>
> @Override
> public void start(Stage stage) throws Exception {
> var textArea = new TextArea();
> var txt = new
> File(getClass().getResource("/Scrollpane.java").getFile());
> textArea.setText(Files.readString(txt.toPath(),
> StandardCharsets.UTF_8));
> var root = new ScrollPane(textArea);
> root.setFitToHeight(true);
> root.setFitToWidth(true);
> Scene scene;
> scene = new Scene(root, 800, 600);
> stage.setTitle("Scroll test");
> stage.setScene(scene);
> stage.show();
> }
>
> public static void main(String[] args) {
> launch(args);
> }
> }
Actually, when I mentioned smooth scrolling, it wasn't performance I was
thinking of, it was the ability to scroll by fractional pixel amounts, but with
the default snap-to-pixel enabled, that won't happen anyway.
I'll look into the other questions you raised tomorrow, specifically what we
might want to do about rendering the cache, and also your question about
snapToPixel.
As for the specific problem with ScrollPane, I found the cause of that one.
There is a (long-standing from the look of it) bug in
[`Region::updateSnappedInsets`](https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/java/javafx/scene/layout/Region.java#L990).
It attempts to account for snap-to-pixel, but does so incorrectly, using
`Math.ceil` without taking screen scaling into account (that is, without
calling the `snapXxxx` methods). The correct code should be something like:
final boolean snap = isSnapToPixel();
snappedTopInset = snapSizeY(insets.getTop(), snap);
snappedRightInset = snapSizeX(insets.getRight(), snap);
snappedBottomInset = snapSizeY(insets.getBottom(), snap);
snappedLeftInset = snapSizeX(insets.getLeft(), snap);
This fixes the ScrollPane problem without needing to modify the cached
rendering code.
-
PR: https://git.openjdk.java.net/jfx/pull/308
Re: RFR: 8242361: JavaFX Web View crashes with Segmentation Fault, when HTML contains Data-URIs [v2]
On Sun, 13 Dec 2020 16:09:16 GMT, Matthias Bläsing
wrote:
>> The code in WTF::scheduleDispatchFunctionsOnMainThread assumes, that
>> the java class com.sun.webkit.MainThread can be found be the JNI
>> function FindClass. This is only true if the class is loadable by the
>> system class loader.
>>
>> One such case is when the OpenJFX modules are loaded from a new
>> ModuleLayer. To fix this, the reference to the class needs to be loaded
>> from when a JNI call from Java into native code is active. In that case
>> FindClass uses the classloader associated with that method.
>>
>> The test code can be executed by running:
>>
>> cd tests/manual/web/dataurl
>> ../../../../gradlew run
>
> Matthias Bläsing has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8242361: Add missing copyright headers and integrate test into systemTests
The fix looks good to me, although I want Arun to look at it, too.
The test looks pretty good with a few comments that I added.
modules/javafx.web/src/main/native/Source/WTF/wtf/java/MainThreadJava.cpp line
51:
> 49: // initilization has to be done from a context where the class
> 50: // com.sun.webkit.MainThread is accessible. When
> 51: // scheduleDispatchFunctionsOnMainThread is invoced, the system class
> loader
Minor typo: invoced --> invoked
modules/javafx.web/src/main/native/Source/WTF/wtf/java/MainThreadJava.cpp line
64:
> 62: // WebPage will be used by FindClass.
> 63: //
> 64: // WTF::initializeMainThread has a guard, so that initialization is
> only run
I guess you meant to say "is only run __once__"?
tests/system/src/test/java/test/com/sun/webkit/MainThreadTest.java line 60:
> 58: final List cmd = asList(
> 59: workerJavaCmd,
> 60: "-cp",appModulePath + "/mymod",
Minor: need space after `,`
tests/system/src/testapp7/java/mymod/myapp7/DataUrlWithModuleLayer.java line
102:
> 100: Platform.runLater(() -> {
> 101: String title = webview.getEngine().getTitle();
> 102: if("Executed".equals(title)) {
Minor: add space after `if`
tests/system/src/test/java/test/com/sun/webkit/MainThreadTest.java line 40:
> 38: */
> 39: public class MainThreadTest {
> 40: @Test
I recommend a `timeout = 15000` in case the launched application doesn't exit.
tests/system/src/testapp7/java/mymod/myapp7/DataUrlWithModuleLayerLauncher.java
line 63:
> 61: Class testClass =
> moduleClassLoader.loadClass("myapp7.DataUrlWithModuleLayer");
> 62: Method launchMethod = appClass.getMethod("launch", Class.class,
> String[].class);
> 63: launchMethod.invoke(null, new Object[]{testClass, args});
You might consider adding a `sleep(15000)` followed by `System.exit(1)` or some
other mechanism to prevent running indefinitely so that if this process hangs
it doesn't hang the test suite (which has happened in the past).
tests/system/src/testapp7/java/mymod/myapp7/DataUrlWithModuleLayer.java line 99:
> 97: @Override
> 98: public void run() {
> 99: while (true) {
Rather than a spin loop in a new thread, it's probably better to wait for the
content to be loaded (checked using a listener on
`WebEngine::getLoadWorker::stateProperty`) before doing the test, which you
shouldn't need to do in a loop. In any case you will want a timeout so that if
this process hangs it doesn't hang the test suite, which has happened in the
past.
-
PR: https://git.openjdk.java.net/jfx/pull/360
Glass backends - xGravity and yGravity
Hi,
Question: Are the xGravity and yGravity still in use?
Windows glass backend seems to ignore it, and I can't think of any scenario
where this would be used on Linux glass backend.
package com.sun.glass.ui;
public abstract class Window {
..
/**
* Sets the window bounds to the specified values.
*
* Gravity values specify how to correct window location if only its size
* changes (for example when stage decorations are added). User initiated
* resizing should be ignored and must not influence window location through
* this mechanism.
*
* The corresponding correction formulas are:
*
* {@code x -= xGravity * deltaW}
* {@code y -= yGravity * deltaH}
*
* @param x the new window horizontal position, ignored if xSet is set to
* false
* @param y the new window vertical position, ignored if ySet is set to
* false
* @param xSet indicates whether the x parameter is valid
* @param ySet indicates whether the y parameter is valid
* @param w the new window width, ignored if set to -1
* @param h the new window height, ignored if set to -1
* @param cw the new window content width, ignored if set to -1
* @param ch the new window content height, ignored if set to -1
* @param xGravity the xGravity coefficient
* @param yGravity the yGravity coefficient
*/
public void setBounds(float x, float y, boolean xSet, boolean ySet,
float w, float h, float cw, float ch,
float xGravity, float yGravity)
Re: RFR: 8242621: TabPane: Memory leak when switching skin
On Tue, 13 Oct 2020 15:03:26 GMT, Jeanette Winzenburg wrote: >> `TabPaneSkin` installs some listeners that are not removed when >> `TabPaneSkin` is changed. >> The fix converts listeners to WeakListeners and also removes them on dispose. >> >> There is a NPE check change needed in `isHosrizontal()`. Without this check >> it causes NPE if pulse is in progress while TabPaneSkin is getting disposed. >> >> `SkinMemoryLeakTest` already had a test which only needed to be enabled. >> Test fails before and passes after this change. > > no review yet, just a couple of quick comments from my experience on recent > cleanup of skins: > > - if NPE checks seem to be necessary, they always indicate an illegal state: > whatever a method is doing, it must not access the skin after dispose. > Usually it's the caller of the method that's misbehaving, it simply must not > happen. It's worth digging why _exactly_ that's happening and if/how it can > be solved without guarding against the null > - while the overall memory test is already done, we still must test every > single skin against side-effects (f.i. of listeners not doing any "late" > update due to not being yet removed - the NPE above could well be such a > case). Have a look at SkinCleanupTest for examples > - when changing listener wiring, it's often a good idea to test that they are > still doing there job (if not yet covered in the available tests) > > yeah, you don't get away with not writing tests *good-humored-grinning quick question: is this still in the lane for fx16? - PR: https://git.openjdk.java.net/jfx/pull/318
Integrated: 8256983: GitHub actions: specify the version of each platform OS and compiler
On Tue, 8 Dec 2020 20:06:21 GMT, Kevin Rushforth wrote: > As described in the [JBS > issue](https://bugs.openjdk.java.net/browse/JDK-8256983), we should specify > the specific version of each OS and compiler rather than just using the > defaults. This will help insulate us from changes to the defaults that can > break the build, and has recently done so. > > On Linux, we upgraded to 20.04 (18.04 is still the default), which required > specifying the version of ant, since the default version for 20.04 is 1.10.7 > which has a bug that causes FX apps to fail. I decided to also specify the > version of ant (1.10.5) for all three platforms. > > The following will be used: > > | Platform | OS | Compiler | Ant | > | --- | --- | --- | --- | > | Linux | Ubuntu 20.04 | gcc 10.2 | ant 1.10.5 | > | Mac | macOS 10.15 | Xcode 11.3.1 | ant 1.10.5 | > | Windows | Windows Server 2019 | [1] | ant 1.10.5 | > > [1] The Microsoft compiler version is unspecified as there is a dependency on > [JDK-8255713](https://bugs.openjdk.java.net/browse/JDK-8255713) in addition > to the problem of increasing the build time to download a specific version. This pull request has now been integrated. Changeset: f2928d95 Author:Kevin Rushforth URL: https://git.openjdk.java.net/jfx/commit/f2928d95 Stats: 46 lines in 1 file changed: 34 ins; 0 del; 12 mod 8256983: GitHub actions: specify the version of each platform OS and compiler Reviewed-by: arapte - PR: https://git.openjdk.java.net/jfx/pull/366
Re: RFR: 8211294: [windows] TextArea content is blurry with 125% scaling
On Sat, 12 Dec 2020 22:31:56 GMT, Kevin Rushforth wrote:
>
>
> One more comment: given the quality problems that necessarily arise when the
> translation of a cached image is not on an integer boundary, part of the
> solution might be to snap the cached image to a pixel boundary as is done in
> this PR, but we would need to ensure that this doesn't impact smooth
> scrolling of a TextArea.
I've done some quick tests with the minimal sample below, and could not notice
any performance impact with this patch compared to 15.0.1.
java
public class TextScroll extends Application {
@Override
public void start(Stage stage) throws Exception {
var textArea = new TextArea();
var txt = new
File(getClass().getResource("/Scrollpane.java").getFile());
textArea.setText(Files.readString(txt.toPath(),
StandardCharsets.UTF_8));
var root = new ScrollPane(textArea);
root.setFitToHeight(true);
root.setFitToWidth(true);
Scene scene;
scene = new Scene(root, 800, 600);
stage.setTitle("Scroll test");
stage.setScene(scene);
stage.show();
}
public static void main(String[] args) {
launch(args);
}
}
-
PR: https://git.openjdk.java.net/jfx/pull/308
Re: Linux basic application not displaying correctly
Hi Chad, I did not get the issue, please send the screenshots to my e-mail as they don't get through the list. One possibility: The Gtk glass backend tries to follow Windows (as in the operating system) sizes. On Windows, window sizes are the whole window, including decorations. On X (Linux) window decorations are a window manager job, so they don't count to the window size (more logical to me, as decorations can vary). Gtk backend tries to mimic that by getting decoration sizes. Your window manager seems to be not sending those. Cheers. Em dom., 13 de dez. de 2020 às 16:58, Chad Preisler escreveu: > Hello, > I'm running elementaryOS and when I use gtk3 the main window does not > display correctly. When using gtk2, the main window does display correctly. > > Here is the command line to run with gtk2. > > java -Djdk.gtk.version=2 > --module-path="/home/chad/javafx/jfx/build/sdk/lib" --add-modules > javafx.controls,javafx.fxml -jar target/javafxElementaryOS-1.0-SNAPSHOT.jar > > Here is the command line to run with gtk3. > > java -Djdk.gtk.version=2 > --module-path="/home/chad/javafx/jfx/build/sdk/lib" --add-modules > javafx.controls,javafx.fxml -jar target/javafxElementaryOS-1.0-SNAPSHOT.jar > > I've added two screenshots. Please notice in the gtk3 one that the mouse > cursor changes inside the window. I can actually change the window size > from that spot. > > I modified glass_window.cpp and added some cout statements to print > messages. The major difference I see in method > WindowContextTop::process_configure the frame width and height returned > from gdk_window_get_frame_extents are bigger in the gtk3 run. > > Here is my output from the gtk2 run: > > chad@chad-Inspiron-5565:~/NetBeansProjects/javafxElementaryOS$ java > -Djdk.gtk.version=2 --module-path="/home/chad/javafx/jfx/build/sdk/lib" > --add-modules javafx.controls,javafx.fxml -jar > target/javafxElementaryOS-1.0-SNAPSHOT.jar > Gtk-Message: 13:49:22.237: Failed to load module "canberra-gtk-module" > Creating main window! POPUP2 > WindowType is not TITLED > set_bounds > Frame extents: top 0 left 0 bottom 0 right 0 > geometry_get_window_width 640 > geometry_get_content_width 640 > geometry_get_window_height 513 > geometry_get_content_width 480 > window_configure > window_configure newWidth 640 newHeight 480 > geometry_get_window_width 640 > geometry_get_window_height 513 > set_bounds > geometry_get_window_width 640 > geometry_get_window_height 513 > window_configure > process_configure > Window is decorated > frame.width 640 frame.height 480 > Frame x 640 frame y 209 > Frame extents: top 0 left 0 bottom 0 right 0 > geometry_get_window_width 640 > geometry_get_window_height 513 > process_configure > Window is decorated > frame.width 640 frame.height 513 > Frame x 640 frame y 209 > Frame extents: top 33 left 0 bottom 0 right 0 > geometry_get_window_width 640 > geometry_get_window_height 513 > process_configure > Window is decorated > frame.width 640 frame.height 513 > Frame x 640 frame y 209 > Frame extents: top 33 left 0 bottom 0 right 0 > geometry_get_window_width 640 > geometry_get_window_height 513 > process_configure > Window is decorated > frame.width 640 frame.height 513 > Frame x 640 frame y 283 > Frame extents: top 33 left 0 bottom 0 right 0 > geometry_get_window_width 640 > geometry_get_window_height 513 > > Here is my output from the gtk3 run: > > chad@chad-Inspiron-5565:~/NetBeansProjects/javafxElementaryOS$ java > -Djdk.gtk.version=3 --module-path="/home/chad/javafx/jfx/build/sdk/lib" > --add-modules javafx.controls,javafx.fxml -jar > target/javafxElementaryOS-1.0-SNAPSHOT.jar > Creating main window! POPUP2 > WindowType is not TITLED > set_bounds > Frame extents: top 0 left 0 bottom 0 right 0 > geometry_get_window_width 640 > geometry_get_content_width 640 > geometry_get_window_height 513 > geometry_get_content_width 480 > window_configure > window_configure newWidth 640 newHeight 480 > geometry_get_window_width 640 > geometry_get_window_height 513 > set_bounds > geometry_get_window_width 640 > geometry_get_window_height 513 > window_configure > process_configure > Window is decorated > frame.width 798 frame.height 671 > Frame x 561 frame y 144 > Frame extents: top 0 left 0 bottom 0 right 0 > geometry_get_window_width 640 > geometry_get_window_height 513 > > I'm pretty sure this is some issue with the Window manager that > elemenatryOS uses. Problem is I'm not sure where to go from here. Has > anyone else experienced this? Any ideas on how to further debug this? > > Thanks, > Chad > [image: javafx_gtk2.png] > [image: javafx_gtk3.png] >
Re: RFR: 8211294: [windows] TextArea content is blurry with 125% scaling
On Sat, 12 Dec 2020 22:31:56 GMT, Kevin Rushforth wrote:
>> I spent a bit of time looking at this. I think the root cause of the problem
>> is in ScrollPane itself. It is attempting to layout its children by doing a
>> snap to pixel (meaning that the final scaled translation should be an
>> integer value), but it is failing to do so. This is mostly not a problem
>> when caching is disabled, since our text rendering does sub-pixel
>> antialiasing that looks crisp even at non-integer boundaries. However,
>> translating an already-rendered image by a non-integer boundary will cause
>> the blurriness we are seeing. There is another issue with the Y translation
>> which isn't 0 even when not using a ScrollPane.
>>
>> I'll continue looking at this in the coming week.
>
> One more comment: given the quality problems that necessarily arise when the
> translation of a cached image is not on an integer boundary, part of the
> solution might be to snap the cached image to a pixel boundary as is done in
> this PR, but we would need to ensure that this doesn't impact smooth
> scrolling of a TextArea.
Further investigations on my part raised one more question, which hopefully you
can answer:
To which extend should `setSnapToPixel` ensure children of a region are indeed
snap to whole pixel coordinates?
To make it clearer, please consider the following sample:
java
public class Blur extends Application {
@Override
public void start(final Stage stage) throws Exception {
var root = new Pane();
root.setSnapToPixel(true);
var ctrl = new CheckBox("Cached");
ctrl.setLayoutX(0.5);
ctrl.setLayoutY(0.5);
ctrl.cacheProperty().bind(ctrl.selectedProperty());
ctrl.setSelected(true);
var ctrl2 = new Button("Foo");
ctrl2.setLayoutX(0.5);
ctrl2.setLayoutY(30.5);
ctrl2.cacheProperty().bind(ctrl.selectedProperty());
var ctrl3 = new Button("Bar");
ctrl3.setLayoutY(60);
ctrl3.cacheProperty().bind(ctrl.selectedProperty());
root.getChildren().addAll(ctrl, ctrl2, ctrl3);
Scene scene;
scene = new Scene(root, 200, 200);
stage.setTitle("Blur test");
stage.setScene(scene);
stage.show();
}
public static void main(String[] args) {
launch(args);
}
}
In this sample, LayoutX and Y properties are deliberately set to non integer
values for the first two controls (the last one serves as a visual baseline,
but `snapToPixel` is set to true. Also clicking the check box toggles caching
for all controls.
Here's what it looks looks **at 100% scaling**, (OpenJFX 15.0.1), with cache
enabled:
and with cache disabled:
What is the legitimate result to expect here; should
`root.setSnapToPixel(true);` override `setLayoutX(0.5);` and align everything
for crisp rendering (as is my understanding)? Or am I misunderstanding the
scope of `setSnapToPixel` and it has no effect when layout is set explicitly?
-
PR: https://git.openjdk.java.net/jfx/pull/308
Integrated: 8201568: zForce touchscreen input device fails when closed and immediately reopened
On Sat, 27 Jun 2020 00:21:06 GMT, John Neffenger wrote: > Fixes [JDK-8201568](https://bugs.openjdk.java.net/browse/JDK-8201568). This pull request has now been integrated. Changeset: ebb59e9f Author:John Neffenger Committer: Johan Vos URL: https://git.openjdk.java.net/jfx/commit/ebb59e9f Stats: 4 lines in 1 file changed: 2 ins; 2 del; 0 mod 8201568: zForce touchscreen input device fails when closed and immediately reopened Reviewed-by: jvos - PR: https://git.openjdk.java.net/jfx/pull/258
