Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

8222210: JFXPanel popups open at wrong coordinates when using multiple hidpi monitors #924

Closed
wants to merge 4 commits into from
Closed

8222210: JFXPanel popups open at wrong coordinates when using multiple hidpi monitors #924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0458c74
Transform coordinates using platform scale if needed when bridging from
johanvos Oct 20, 2022
af931be
address reviewer comments
johanvos Oct 28, 2022
783c45a
Process reviewer comments
johanvos Nov 2, 2022
4dd021c
Process reviewer comments. Use Math.round() instead of Math.floor() f…
johanvos Nov 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions modules/javafx.graphics/src/main/java/module-info.java
View file
Open in desktop
Expand Up @@ -68,6 +68,7 @@

exports com.sun.glass.ui to
javafx.media,
javafx.swing,
javafx.web;
exports com.sun.glass.utils to
javafx.media,
Expand Down
58 changes: 53 additions & 5 deletions modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java
View file
Open in desktop
Expand Up @@ -38,6 +38,8 @@
import java.awt.EventQueue;
import java.awt.SecondaryLoop;
import java.awt.GraphicsEnvironment;
import java.awt.GraphicsConfiguration;
import java.awt.Rectangle;
import java.awt.event.AWTEventListener;
import java.awt.event.ComponentEvent;
import java.awt.event.FocusEvent;
Expand All @@ -48,6 +50,7 @@
import java.awt.event.MouseEvent;
import java.awt.event.MouseWheelEvent;
import java.awt.event.InvocationEvent;
import java.awt.geom.AffineTransform;
import java.awt.im.InputMethodRequests;
import java.awt.image.BufferedImage;
import java.awt.image.DataBufferInt;
Expand All @@ -59,7 +62,9 @@
import javax.swing.SwingUtilities;

import javafx.application.Platform;
import javafx.geometry.Point2D;
import javafx.scene.Scene;
import com.sun.glass.ui.Screen;

import com.sun.javafx.application.PlatformImpl;
import com.sun.javafx.cursor.CursorFrame;
Expand Down Expand Up @@ -362,6 +367,44 @@ public final boolean isOpaque() {
return false;
}

// we need to know the JavaFX screen of the current AWT graphcisConfiguration
private Screen findScreen(GraphicsConfiguration graphicsConfiguration) {
Rectangle awtBounds = graphicsConfiguration.getBounds();
AffineTransform awtScales = graphicsConfiguration.getDefaultTransform();
for (Screen screen : Screen.getScreens()) {
if ((Math.abs(screen.getPlatformX() - awtBounds.getX()) < 2.) &&
(Math.abs(screen.getPlatformY() - awtBounds.getY()) < 2.) &&
(Math.abs(screen.getPlatformWidth() - awtScales.getScaleX() * awtBounds.getWidth()) < 2.) &&
(Math.abs(screen.getPlatformHeight() - awtScales.getScaleY() * awtBounds.getHeight()) < 2.)) {
return screen;
}
}
return null;
}

private Point2D convertSwingToFxPixel(GraphicsConfiguration g, double wx, double wy) {
double newx, newy;
Screen screen = findScreen(g);
if (screen != null) {
AffineTransform awtScales = getGraphicsConfiguration().getDefaultTransform();
float pScaleX = screen.getPlatformScaleX();
float pScaleY = screen.getPlatformScaleY();
int sx = screen.getX();
int sy = screen.getY();
double awtScaleX = awtScales.getScaleX();
double awtScaleY = awtScales.getScaleY();

int px = screen.getPlatformX();
int py = screen.getPlatformY();
newx = sx + (wx - px) * awtScaleX / pScaleX;
newy = sy + (wy - py) * awtScaleY / pScaleY;
Comment on lines +399 to +400
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

px and py are already (scaled) "FX" coordinates, so I'm not sure AWT scale should be applied to that. I might be missing something here.

Copy link
Collaborator Author

@johanvos johanvos Nov 2, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this correction, there is an offset on Linux (when awtScaleX = 1 and pScaleX =2 for example, versus 1.5 for both on Windows).
TBH, I think the relations between the different scale factors should be written out clearly somewhere, as I think there are a number of places where we face platform-specific differences.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree this should be more clearly documented. I've tested it on macOS and Windows and it works fine, so I expect that this is correct as you have it.

} else {
newx = wx;
newy = wy;
}
return new Point2D(newx, newy);
}

private void sendMouseEventToFX(MouseEvent e) {
if (scenePeer == null || !isFxEnabled()) {
return;
Expand Down Expand Up @@ -407,14 +450,17 @@ private void sendMouseEventToFX(MouseEvent e) {
if (e.getID() == MouseEvent.MOUSE_PRESSED || e.getID() == MouseEvent.MOUSE_RELEASED) {
popupTrigger = e.isPopupTrigger();
}
Point2D onScreen = convertSwingToFxPixel(getGraphicsConfiguration(), e.getXOnScreen(), e.getYOnScreen());
int fxXOnScreen = (int)Math.round(onScreen.getX());
int fxYOnScreen = (int)Math.round(onScreen.getY());

if(e.getID() == MouseEvent.MOUSE_WHEEL) {
scenePeer.scrollEvent(AbstractEvents.MOUSEEVENT_VERTICAL_WHEEL,
0, -SwingEvents.getWheelRotation(e),
0, 0, // total scroll
40, 40, // multiplier
e.getX(), e.getY(),
e.getXOnScreen(), e.getYOnScreen(),
fxXOnScreen, fxYOnScreen,
(extModifiers & MouseEvent.SHIFT_DOWN_MASK) != 0,
(extModifiers & MouseEvent.CTRL_DOWN_MASK) != 0,
(extModifiers & MouseEvent.ALT_DOWN_MASK) != 0,
Expand All @@ -425,15 +471,16 @@ private void sendMouseEventToFX(MouseEvent e) {
SwingEvents.mouseButtonToEmbedMouseButton(e.getButton(), extModifiers),
primaryBtnDown, middleBtnDown, secondaryBtnDown,
backBtnDown, forwardBtnDown,
e.getX(), e.getY(), e.getXOnScreen(), e.getYOnScreen(),
e.getX(), e.getY(),
fxXOnScreen, fxYOnScreen,
(extModifiers & MouseEvent.SHIFT_DOWN_MASK) != 0,
(extModifiers & MouseEvent.CTRL_DOWN_MASK) != 0,
(extModifiers & MouseEvent.ALT_DOWN_MASK) != 0,
(extModifiers & MouseEvent.META_DOWN_MASK) != 0,
popupTrigger);
}
if (e.isPopupTrigger()) {
scenePeer.menuEvent(e.getX(), e.getY(), e.getXOnScreen(), e.getYOnScreen(), false);
scenePeer.menuEvent(e.getX(), e.getY(), fxXOnScreen, fxYOnScreen, false);
}
}

Expand Down Expand Up @@ -598,8 +645,9 @@ private boolean updateScreenLocation() {
synchronized (getTreeLock()) {
if (isShowing()) {
Point p = getLocationOnScreen();
screenX = p.x;
screenY = p.y;
Point2D fxcoord = convertSwingToFxPixel(getGraphicsConfiguration(), p.x, p.y);
screenX = (int)Math.round(fxcoord.getX());
screenY = (int)Math.round(fxcoord.getY());
return true;
}
}
Expand Down