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

8280861: Robot color picker broken on Linux with scaling above 100% #7425

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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,5 +1,5 @@
/*
* Copyright (c) 1999, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -334,7 +334,7 @@ Java_sun_awt_X11_XRobotPeer_getRGBPixelsImpl( JNIEnv *env,
if (useGtk) {
gtk->gdk_threads_enter();
gtk_failed = gtk->get_drawable_data(env, pixelArray, x, y, width,
height, jwidth, dx, dy, 1);
height, jwidth, dx, dy);
gtk->gdk_threads_leave();
}

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -2471,27 +2471,14 @@ static jobject gtk2_get_setting(JNIEnv *env, Setting property)
}

static gboolean gtk2_get_drawable_data(JNIEnv *env, jintArray pixelArray, jint x,
jint y, jint width, jint height, jint jwidth, int dx, int dy, jint scale) {
jint y, jint width, jint height, jint jwidth, int dx, int dy) {
GdkPixbuf *pixbuf;
jint *ary;

GdkWindow *root = (*fp_gdk_get_default_root_window)();

pixbuf = (*fp_gdk_pixbuf_get_from_drawable)(NULL, root, NULL, x, y,
0, 0, width, height);
if (pixbuf && scale != 1) {
GdkPixbuf *scaledPixbuf;
x /= scale;
y /= scale;
width /= scale;
height /= scale;
dx /= scale;
dy /= scale;
scaledPixbuf = (*fp_gdk_pixbuf_scale_simple)(pixbuf, width, height,
GDK_INTERP_BILINEAR);
(*fp_g_object_unref)(pixbuf);
pixbuf = scaledPixbuf;
}

if (pixbuf) {
int nchan = (*fp_gdk_pixbuf_get_n_channels)(pixbuf);
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -37,7 +37,9 @@
#include "sizecalc.h"
#include <jni_util.h>
#include <stdio.h>
#include <math.h>
#include "awt.h"
#include "debug_assert.h"

static void *gtk3_libhandle = NULL;
static void *gthread_libhandle = NULL;
@@ -2861,35 +2863,48 @@ static void transform_detail_string (const gchar *detail,
}
}

inline static int scale_down_ceiling(int what, int scale) {
return (int)ceilf(what / (float)scale);
Copy link
Contributor

@prrace prrace Feb 15, 2022

Choose a reason for hiding this comment

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

I find the name of this and the companion function confusing.
what is "inf" short for ? I see that and I think plus_infinity.
scaled_ceiling and scaled_floor might be better names based on what I see.

Copy link
Member Author

@mkartashev mkartashev Feb 15, 2022

Choose a reason for hiding this comment

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

Right, "inf" is for infinity; this is sort of how the rounding modes are described in the IEEE754 standard. But since full length words make the function name too long and short versions are confusing, I will rename as you suggested.

Copy link
Member Author

@mkartashev mkartashev Feb 15, 2022

Choose a reason for hiding this comment

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

@prrace The change has been made, please, take a look.

}

inline static int scale_down_floor(int what, int scale) {
return (int)floorf(what / (float)scale);
}

static gboolean gtk3_get_drawable_data(JNIEnv *env, jintArray pixelArray,
int x, jint y, jint width, jint height, jint jwidth, int dx, int dy,
jint scale) {
int x, jint y, jint width, jint height, jint jwidth, int dx, int dy) {
GdkPixbuf *pixbuf;
jint *ary;

int skip_left = 0;
int skip_top = 0;
GdkWindow *root = (*fp_gdk_get_default_root_window)();
if (gtk3_version_3_10) {
int win_scale = (*fp_gdk_window_get_scale_factor)(root);

// Scale the coordinate and size carefully such that the captured area
// is at least as large as requested. We trim off excess later by
// using the skip_* variables.
const int x_scaled = scale_down_floor(x, win_scale);
const int y_scaled = scale_down_floor(y, win_scale);
skip_left = x - x_scaled*win_scale;
skip_top = y - y_scaled*win_scale;
DASSERT(skip_left >= 0 && skip_top >= 0);

const int x_right_scaled = scale_down_ceiling(x + width, win_scale);
const int width_scaled = x_right_scaled - x_scaled;
DASSERT(width_scaled > 0);

const int y_bottom_scaled = scale_down_ceiling(y + height, win_scale);
const int height_scaled = y_bottom_scaled - y_scaled;
DASSERT(height_scaled > 0);

pixbuf = (*fp_gdk_pixbuf_get_from_drawable)(
root, x, y, (int) (width / (float) win_scale + 0.5), (int) (height / (float) win_scale + 0.5));
root, x_scaled, y_scaled, width_scaled, height_scaled);
} else {
pixbuf = (*fp_gdk_pixbuf_get_from_drawable)(root, x, y, width, height);
}

if (pixbuf && scale != 1) {
GdkPixbuf *scaledPixbuf;
x /= scale;
y /= scale;
width /= scale;
height /= scale;
dx /= scale;
dy /= scale;
scaledPixbuf = (*fp_gdk_pixbuf_scale_simple)(pixbuf, width, height,
GDK_INTERP_BILINEAR);
(*fp_g_object_unref)(pixbuf);
pixbuf = scaledPixbuf;
}

if (pixbuf) {
int nchan = (*fp_gdk_pixbuf_get_n_channels)(pixbuf);
int stride = (*fp_gdk_pixbuf_get_rowstride)(pixbuf);
@@ -2906,7 +2921,8 @@ static gboolean gtk3_get_drawable_data(JNIEnv *env, jintArray pixelArray,
int index;
for (_y = 0; _y < height; _y++) {
for (_x = 0; _x < width; _x++) {
p = pix + (intptr_t) _y * stride + _x * nchan;
p = pix + (intptr_t) (_y + skip_top) * stride
+ (_x + skip_left) * nchan;

index = (_y + dy) * jwidth + (_x + dx);
ary[index] = 0xff000000
@@ -510,7 +510,7 @@ typedef struct GtkApi {
guint32 timestamp, GError **error);
gboolean (*get_drawable_data)(JNIEnv *env, jintArray pixelArray,
jint x, jint y, jint width, jint height,
jint jwidth, int dx, int dy, jint scale);
jint jwidth, int dx, int dy);
void (*g_free)(gpointer mem);


@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -27,20 +27,26 @@
import java.awt.Frame;
import java.awt.Graphics;
import java.awt.Panel;
import java.awt.Point;
import java.awt.Rectangle;
import java.awt.Robot;
import java.awt.image.BufferedImage;
import javax.swing.UIManager;
import javax.imageio.ImageIO;
import java.io.File;
import java.io.IOException;

/**
* @test
* @key headful
* @bug 8073320
* @summary Windows HiDPI support
* @bug 8073320 8280861
* @summary Linux and Windows HiDPI support
* @author Alexander Scherbatiy
* @requires (os.family == "linux" | os.family == "windows")
* @run main/othervm -Dsun.java2d.win.uiScaleX=3 -Dsun.java2d.win.uiScaleY=2
* HiDPIRobotScreenCaptureTest
* @run main/othervm -Dsun.java2d.uiScale=1 HiDPIRobotScreenCaptureTest
* @run main/othervm -Dsun.java2d.uiScale=2 HiDPIRobotScreenCaptureTest
*/

public class HiDPIRobotScreenCaptureTest {
@@ -60,7 +66,14 @@ public static void main(String[] args) throws Exception {
}

Frame frame = new Frame();
frame.setBounds(40, 30, 400, 300);
// Position the frame on prime number coordinates to avoid
// them being multiple of the desktop scale; this tests Linux
// color picker better.
// Also, the position should be far enough from the top left
// corner of the screen to reduce the chance of being repositioned
// by the system because that area's occupied by the global
// menu bar and such.
frame.setBounds(83, 97, 400, 300);
frame.setUndecorated(true);

Panel panel = new Panel(new BorderLayout());
@@ -86,11 +99,12 @@ public void paint(Graphics g) {
frame.setVisible(true);
Robot robot = new Robot();
robot.waitForIdle();
Thread.sleep(200);
robot.delay(500);

Rectangle rect = canvas.getBounds();
rect.setLocation(canvas.getLocationOnScreen());

System.out.println("Creating screen capture of " + rect);
BufferedImage image = robot.createScreenCapture(rect);
frame.dispose();

@@ -101,20 +115,38 @@ public void paint(Graphics g) {
throw new RuntimeException("Wrong image size!");
}

if (image.getRGB(w / 4, h / 4) != COLORS[0].getRGB()) {
throw new RuntimeException("Wrong image color!");
}

if (image.getRGB(3 * w / 4, h / 4) != COLORS[1].getRGB()) {
throw new RuntimeException("Wrong image color!");
}
checkRectColor(image, new Rectangle(0, 0, w / 2, h / 2), COLORS[0]);
checkRectColor(image, new Rectangle(w / 2, 0, w / 2, h / 2), COLORS[1]);
checkRectColor(image, new Rectangle(0, h / 2, w / 2, h / 2), COLORS[2]);
checkRectColor(image, new Rectangle(w / 2, h / 2, w / 2, h / 2), COLORS[3]);
}

if (image.getRGB(w / 4, 3 * h / 4) != COLORS[2].getRGB()) {
throw new RuntimeException("Wrong image color!");
}
private static final int OFFSET = 5;
static void checkRectColor(BufferedImage image, Rectangle rect, Color expectedColor) {
System.out.println("Checking rectangle " + rect + " to have color " + expectedColor);
final Point[] pointsToCheck = new Point[] {
new Point(rect.x + OFFSET, rect.y + OFFSET), // top left corner
new Point(rect.x + rect.width - OFFSET, rect.y + OFFSET), // top right corner
new Point(rect.x + rect.width / 2, rect.y + rect.height / 2), // center
new Point(rect.x + OFFSET, rect.y + rect.height - OFFSET), // bottom left corner
new Point(rect.x + rect.width - OFFSET, rect.y + rect.height - OFFSET) // bottom right corner
};

if (image.getRGB(3 * w / 4, 3 * h / 4) != COLORS[3].getRGB()) {
throw new RuntimeException("Wrong image color!");
for (final var point : pointsToCheck) {
System.out.print("Checking color at " + point + " to be equal to " + expectedColor);
final int actualColor = image.getRGB(point.x, point.y);
if (actualColor != expectedColor.getRGB()) {
System.out.println("... Mismatch: found " + new Color(actualColor) + " instead. Check image.png.");
try {
ImageIO.write(image, "png", new File("image.png"));
} catch(IOException e) {
System.out.println("failed to save image.png.");
e.printStackTrace();
}
throw new RuntimeException("Wrong image color!");
} else {
System.out.println("... OK");
}
}
}
}
}
@@ -0,0 +1,111 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, JetBrains s.r.o.. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

import java.awt.BorderLayout;
import java.awt.Canvas;
import java.awt.Color;
import java.awt.Frame;
import java.awt.Graphics;
import java.awt.Panel;
import java.awt.Point;
import java.awt.Rectangle;
import java.awt.Robot;

/**
* @test
* @key headful
* @bug 8280861
* @summary Verifies Robot screen capture capabilities with different
* Gtk backends and presence of UI scaling
* @requires os.family == "linux"
* @run main/othervm -Djdk.gtk.version=2 -Dsun.java2d.uiScale=1 ScreenCaptureGtkTest
* @run main/othervm -Djdk.gtk.version=2 -Dsun.java2d.uiScale=2 ScreenCaptureGtkTest
* @run main/othervm -Djdk.gtk.version=2 -Dsun.java2d.uiScale=3 ScreenCaptureGtkTest
* @run main/othervm -Djdk.gtk.version=3 -Dsun.java2d.uiScale=1 ScreenCaptureGtkTest
* @run main/othervm -Djdk.gtk.version=3 -Dsun.java2d.uiScale=2 ScreenCaptureGtkTest
* @run main/othervm -Djdk.gtk.version=3 -Dsun.java2d.uiScale=3 ScreenCaptureGtkTest
*/

public class ScreenCaptureGtkTest {
private static final Color[] COLORS = {
Color.GREEN, Color.BLUE, Color.ORANGE, Color.RED};

public static void main(String[] args) throws Exception {
Frame frame = new Frame();
// Position the frame on prime number coordinates to avoid
// them being multiple of the desktop scale; this tests Linux
// color picker better.
// Also, the position should be far enough from the top left
// corner of the screen to reduce the chance of being repositioned
// by the system because that area's occupied by the global
// menu bar and such.
frame.setBounds(83, 97, 400, 300);
frame.setUndecorated(true);

Panel panel = new Panel(new BorderLayout());
Canvas canvas = new Canvas() {
@Override
public void paint(Graphics g) {
super.paint(g);
int w = getWidth();
int h = getHeight();
g.setColor(COLORS[0]);
g.fillRect(0, 0, w, h);
// Paint several distinct pixels next to one another
// in order to test color picker's precision.
for (int i = 1; i < 4; i++) {
g.setColor(COLORS[i]);
g.fillRect(i, 0, 1, 1);
}
}
};

panel.add(canvas);
frame.add(panel);
frame.setVisible(true);
Robot robot = new Robot();
robot.waitForIdle();
robot.delay(500);

final Point screenLocation = frame.getLocationOnScreen();
checkPixelColors(robot, screenLocation.x, screenLocation.y);

robot.delay(100);
frame.dispose();
}

static void checkPixelColors(Robot robot, int x, int y) {
for (int i = 0; i < 4; i++) {
final Color actualColor = robot.getPixelColor(x + i, y);
System.out.print("Checking color at " + (x + i) + ", " + y + " to be equal to " + COLORS[i]);
if (!actualColor.equals(COLORS[i])) {
System.out.println("... Mismatch: found " + actualColor + " instead");
throw new RuntimeException("Wrong screen pixel color");

} else {
System.out.println("... OK");
}
}
}
}