JDK-8015739: Background of JInternalFrame is located out of JInternalFrame #10274

honkar-jdk · 2022-09-14T18:33:47Z

JInternalFrame background color seems to overflow into the border region. This issue is more prominently seen on Windows - Metal LAF (with fractional scaling, as shown below). The primary reason is border scaling issue as observed in - JDK-8279614

The fix involves a similar approach as described here #7449 (comment). The test checks the midpoint and corners of borders to check if the internal frame's background color is located out of JInternalFrame.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8015739: Background of JInternalFrame is located out of JInternalFrame

Reviewers

Alexander Zuev (@azuev-java - Reviewer)
Alexey Ivanov (@aivanov-jdk - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10274/head:pull/10274
$ git checkout pull/10274

Update a local copy of the PR:
$ git checkout pull/10274
$ git pull https://git.openjdk.org/jdk pull/10274/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10274

View PR using the GUI difftool:
$ git pr show -t 10274

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10274.diff

bridgekeeper · 2022-09-14T18:34:49Z

👋 Welcome back honkar! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-09-14T18:42:35Z

@honkar-jdk The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2022-09-14T18:47:32Z

Webrevs

azuev-java · 2022-09-19T19:42:29Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

@@ -217,16 +239,27 @@ public Insets getBorderInsets(Component c, Insets newInsets) {
     */
    @SuppressWarnings("serial") // Superclass is not serializable across versions
    public static class InternalFrameBorder extends AbstractBorder implements UIResource {
-        private static final int corner = 14;
+        private static int corner = 14;


Before it was final, now it is not and it is static - means you are changing it down the line. What will happen if two instances will try to change it in a different ways - i.e. if there are more than one window and they are on a screens with different scale factors? I do not really get the logic - either this value is a local one and being calculated for each instance or it is a final and constant for all. And seeing how you update it continuously - will it not accumulate this change is paintBorder will be called multiple times?

@azuev-java Thank you for reviewing and bringing up - "multiple windows on multiple screens with different scaling" scenario. I missed accounting for the above scenario in the present fix. Yes, I believe that corner should be local variable that is calculated depending on the screen's scale on which it is displayed.

I was also thinking if we can - may be precompute & store all the corner values in a HashMap for all the scale values that we can expect?

I was also thinking if we can - may be precompute & store all the corner values in a HashMap for all the scale values that we can expect?

It depends on how often this value is being computed and how complicated the computation is. Since we request the current transform value anyways on every paint i do not see how accessing the HashMap would be more beneficial than the simple floating point multiplication and a rounding up the result. Just make the static final constant storing the default corner size and a local variable that stores the calculated current corner. You can try to use HashMap but i doubt it will make much difference performance wise. It is not a complex calculation. I might be wrong so you can try both ways and check for the performance impact.

azuev-java · 2022-09-20T18:17:12Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

-              return newInsets;
-          }
+        public Insets getBorderInsets(Component c, Insets newInsets) {
+          newInsets.set(5, 5, 5, 5);


The default code inset is 4 so this formatting looks incorrect. Plus, i understand that you were trying to better indent the code but usually if code is not changed functionally we avoid changing the lines to make code annotation more informative.

azuev-java · 2022-09-20T18:22:20Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

-              g.drawLine( 0, 1, 0, h-2);
-              g.drawLine( w-1, 1, w-1, h-2);
-              g.drawLine( 1, h-1, w-2, h-1);
+            Graphics2D g2d = (Graphics2D) g;


Regardless, i see that absolutely similar code down this file for FrameBorder and DialogBorder. Are these UI elements suffer from the same issue or is it only limited to InternalFrame? If they do suffer from the same issue i would suggest either fixing it all together or at least submitting new bugs tracking existence of the problem. Of course problem might not be present for some reason in these components but it is worth checking it out while you are working on the code and remember all the details.

@azuev-java Sure. I'll check if there is any observable distortion in other components and either make changes as part of this PR or track it under JDK-8282958. Currently there exists a generic umbrella issue created for border scaling issue, all the related issues are linked to it. The plan is to create a utility method (for common steps) that can be reused across components where this border rendering issue is observed.

@azuev-java Tested FrameBorder and DialogBorder, they have a similar problem to JInternalFrameBorder (border scaling issue). I have created a new JBS issue - JDK-8294484 to track Frame and Dialog Borders of Metal LAF as there is a plan to create a common utility method and refactor the common steps.

Thank you, @honkar-jdk. Once the border handling is unified, fixing FrameBorder and DialogBorder will be easier and code duplication will be avoided.

Refactoring to create a common utility method is being tracked by - JDK-8294680

stanio · 2022-09-21T18:03:08Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

+            // border and corner scaling
+            corner = (int) Math.round(corner * at.getScaleX());


I'm likely missing something from the big picture but wouldn't this accumulate on every paintBorder() invocation (into a static field)?

@stanio Hi, @azuev-java did bring up the same point earlier - #10274 (comment). In the recent commit, I have update the code to use a different local variable - scaledCorner to compute and store the scaled value for each new instance.

azuev-java · 2022-09-29T19:47:43Z

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java

+ *
+ * @run main/othervm -Dsun.java2d.uiScale=1 InternalFrameBorderTest
+ * @run main/othervm -Dsun.java2d.uiScale=2 InternalFrameBorderTest
+ */


Are there any differences for the test instructions on mac and linux? If not then why do we keep two identical test headers instead of one that covers both mac and linux with tag @requires (os.family == "linux" | os.family == "mac") ?

@azuev-java Thanks for catching it. Removed the redundant header.

azuev-java

Looks good to me

openjdk · 2022-09-30T18:38:16Z

@honkar-jdk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8015739: Background of JInternalFrame is located out of JInternalFrame

Reviewed-by: kizune, aivanov

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 599 new commits pushed to the master branch:

b847fb6: 8296414: [BACKOUT] JDK-8295319: pending_cards_at_gc_start doesn't include cards in thread buffers
5b7e706: 8295753: (fs) UnixPath::toRealPath does not return correct case when links not followed
82f9819: 8294536: Update troff form of man page for new --spec-base-url option
b49bdae: 8294816: C2: Math.min/max vectorization miscompilation
c206f28: 8283101: serviceability/jvmti/thread/GetFrameCount/framecnt01/framecnt01.java failing #VirtualThread-Frozen: number of frames expected: 14, got: 9
97c5a64: 8296287: Improve documentation for Types.directSupertypes()
f9c7cda: 8294109: JavaDoc search should search whole index
5622b09: 8200337: Generalize see and link tags for user-defined anchors
22347e4: 8277775: Fixup bugids in RemoveDropTargetCrashTest.java - add 4357905
1231682: 8296305: Remove unimplemented deoptimized_wrt_marked_nmethods
... and 589 more: https://git.openjdk.org/jdk/compare/8ff2c2639e6843333cf220d4427799e21d366764...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@azuev-java, @alisenchung, @aivanov-jdk) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

aivanov-jdk · 2022-10-03T20:47:46Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

+import sun.swing.StringUIClientPropertyKey;
+import sun.swing.SwingUtilities2;
+
+import javax.swing.AbstractButton;
+import javax.swing.ButtonModel;


I'm unsure if we all agree on the order of imports in client area, however, the common order is

import java.*; // optional blank line import javax.*; // blank line import sun.*; // other packages // blank line import static *; // if applicable

aivanov-jdk · 2022-10-03T21:04:37Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

+            g.drawLine( 0, 0, width-1, 0);
+            g.drawLine( 0, 1, 0, height-1);
+            g.drawLine( width-1, 1, width-1, height-1);
+            g.drawLine( 1, height-1, width-1, height-1);


I suggest removing the space after the opening parenthesis.

There's usually a space on either side of binary operators, the - in this case.

I believe this is existing code which was just moved around, so maybe its style is not worth touching.

aivanov-jdk · 2022-10-03T21:18:55Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java


-          }
+            // border and corner scaling
+            int scaledCorner = (int) Math.round(corner * at.getScaleX());


Just a suggestion, you can call this variable corner but make the constant above upper case CORNER. This way the old code would remain basically the same and continue using corner (which was previously a field).

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java

aivanov-jdk · 2022-10-03T21:48:17Z

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java

+    }
+    // for debugging purpose, saves screen capture when test fails.
+    private static void saveScreenCapture(String filename) {
+        BufferedImage image = robot.createScreenCapture(jFrame.getBounds());


I guess we lose pixel details here. If uiScale > 1, createScreenCapture scales down the captured image. Do we want to preserve the original image too? Robot.createMultiResolutionScreenCapture can be useful.

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java

aivanov-jdk · 2022-10-03T21:57:17Z

Out of curiosity, can this test use BufferedImage to render JInternalFrame into?

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

alisenchung · 2022-10-03T22:02:34Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

+            g.drawLine( 1, height-1, width-1, height-1);
+
+            // Draw the bulk of the border
+            for (int i = 1; i <= loopCount; i++) {


Should we redo the drawing of this border to use Graphics2D fillRect instead?

That was my initial thought too. But, as the loopCount won't be really large (Eg. it is 15 for 300% scaling) and it wouldn't affect the performance drastically, I have retained the original approach to draw the bulk of the border.

Let us leave it as is at this time. There are enough changes.

At the same time, it's a good suggestion for optimization. We should consider it in a separate CR.

I agree, this code can be improved further using fillRect() (as a separate PR).

alisenchung · 2022-10-03T22:03:17Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

+            if (c instanceof JInternalFrame && ((JInternalFrame)c).isResizable()) {
+                // Draw the Long highlight lines
+                g.setColor(highlight);
+                g.drawLine(scaledCorner + 1, midPoint+stkWidth,


The spacing here and when drawing shadow lines is a bit inconsistent

@alisenchung I tried by increasing and decreasing the line position a bit, the current combination provided optimal positioning of both the shadow and highlight line within the border for all the scales. The slight inconsistency in spacing is probably due to rounding losses in loop count, stroke width and corner. I'll check it again.

Oh, I meant the actual code spacing, theres a space between scaledCorner and 1, but not space between midPoint and stkWidth

honkar-jdk · 2022-10-03T22:16:56Z

Out of curiosity, can this test use BufferedImage to render JInternalFrame into?

@aivanov-jdk Thank you for reviewing. I wanted to clarify whether you meant saving just the JInternalFrame into BufferedImage?

honkar-jdk · 2022-10-04T18:08:21Z

@aivanov-jdk Regarding saving the screen capture -

When JIF bounds are used, a partial image of JInternalFrame (JIF) is saved. Hence I'm using the entire outer JFrame bounds to capture the screenshot.
For the screenCapture I wanted to use BufferedImage's getScaledInstance() to create the scaled version (code snippet below), but experiencing issues while saving it. Currently I'm scaling the original image and re-drawing it using graphics object.

BufferedImage bufferedImage= robot.createScreenCapture(jFrameBounds);
Image image= bufferedImage.getScaledInstance((int) (scale * bufferedImage.getWidth(null)),
	(int) (scale * bufferedImage.getHeight(null)), Image.SCALE_DEFAULT);

aivanov-jdk · 2022-10-04T18:32:33Z

Out of curiosity, can this test use BufferedImage to render JInternalFrame into?

@aivanov-jdk Thank you for reviewing. I wanted to clarify whether you meant saving just the JInternalFrame into BufferedImage?

Yes, just JInternalFrame. Yet I was talking about rendering rather than capturing from the screen.

aivanov-jdk · 2022-10-04T18:48:19Z

@aivanov-jdk Regarding saving the screen capture -

When JIF bounds are used, a partial image of JInternalFrame (JIF) is saved. Hence I'm using the entire outer JFrame bounds to capture the screenshot.

Could be… It shouldn't. Anyway, I have no problem with capturing the entire JFrame.

For the screenCapture I wanted to use BufferedImage's getScaledInstance() to create the scaled version (code snippet below), but experiencing issues while saving it. Currently I'm scaling the original image and re-drawing it using graphics object.

This is definitely not what we want. It just up-scales or down-scales the current image stored in BufferedImage.

When you run in a HiDPI environment or with uiScale set explicitly to a value greater than 1.0, the number of pixels is higher. Robot.createScreenCapture up-scales the passed in rectangle, captures all the pixels and then down-scales the captured screenshot to the user's space coordinates.

At the same time, createMultiResolutionScreenCapture returns a MultiResolutionImage which contains two variants: (1) the base image with the user specified size, down-scaled from the screen; (2) native resolution image with the device size pixels. The second variant will preserve all the pixels seen on the screen.

aivanov-jdk

It looks good to me, except for the comments I left on the coding style.

Yet there's one thing which I noticed in the rendered border. The top left corner has one pixel which is painted over the title bar. It's present in the original painting code. The top right corner has such a pixel too.

The updated code paints the top left corner with that pixel. Yet in the top right corner, it moved to left.

aivanov-jdk · 2022-10-04T19:28:05Z

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java

+import javax.imageio.ImageIO;
+import javax.swing.JFrame;
+import javax.swing.JInternalFrame;
+import javax.swing.JLabel;
+import javax.swing.SwingUtilities;
+import javax.swing.UIManager;


I'd rather stick to the same order of imports in sources and tests. However, swing components used are more important.

aivanov-jdk · 2022-10-04T19:37:32Z

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java

            if (Color.RED.equals(robot.getPixelColor(
-                    isVertical ? i : (iFrameLoc.x + MIDPOINT),
-                    isHorizontal ? i : (iFrameLoc.y + MIDPOINT)))) {
-                        saveScreenCapture(borderDirection + "_" + uiScale + ".png");
-                        errorLog.append("uiScale: "+ uiScale +
-                                " Red background color" + " detected at "
-                                + borderDirection + " border\n");
+            isVertical ? i : (iFrameLoc.x + MIDPOINT),
+            isHorizontal ? i : (iFrameLoc.y + MIDPOINT)))) {
+                saveScreenCapture(borderDirection + "_" + uiScale + ".png", uiScale);
+                errorLog.append("At uiScale: "+ uiScale +
+                ", Red background color detected at "
+                + borderDirection + " border\n");


This doesn't look right either. The old code had only the body of if incorrectly indented.

if (Color.RED.equals(robot.getPixelColor( isVertical ? i : (iFrameLoc.x + MIDPOINT), isHorizontal ? i : (iFrameLoc.y + MIDPOINT)))) { saveScreenCapture(borderDirection + "_" + uiScale + ".png"); errorLog.append("uiScale: " + uiScale + " Red background color detected at " + borderDirection + " border\n");

Continuation lines are indented twice as much or to align with a level they're wrapped at. The example for the second case:

errorLog.append("uiScale: " + uiScale + " Red background color detected at " + borderDirection + " border\n");

Here, the operators on the wrapped lines align to the following column of the opening parenthesis for parameters of append.

In my example, I put the operators on the continuation line, it's what Java Coding Style recommends. However, leaving the operators on the previous line is very common.

Here, you're mixing two styles: the first continuation line with append parameter had + at the end of the line, but the second line wrapped the + operator to the next line.

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java

aivanov-jdk · 2022-10-04T19:41:42Z

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java

+        Graphics2D g2d = (Graphics2D) scaledImage.getGraphics();
+        g2d.scale(scale, scale);
+        g2d.drawImage(image, 0, 0, null);
+        g2d.dispose();


This does not make sense, it up-scales a down-scaled image. Either revert to the previous version or use createMultiResolutionScreenCapture and save all resolution variants from the MultiResolutionImage.

honkar-jdk · 2022-10-05T21:11:48Z

@aivanov-jdk In addition to review changes, the following changes were made to the test case:

To make the test more robust, corner check has been changed from corner pixel check to checking multiple pixels along the corner diagonals.
Added break statements to the loops to avoid appending multiple error messages within a check.
Saving a single screenshot per uiScale after all the checks are done.

aivanov-jdk · 2022-10-05T21:44:45Z

test/jdk/javax/swing/JInternalFrame/InternalFrameBorderTest.java

-            errorLog.append("At uiScale: "+ uiScale + ", Red background color"
-                    + " detected at " + cornerLocation +  " corner\n");
+
+        for (int i = 0; i < 5; i++) {


Suggested change

for (int i = 0; i < 5; i++) {

for (int i = 0; i < BORDER_THICKNESS; i++) {

Is it the intention?

Yes, that is correct.

Will you update it to use BORDER_THICKNESS?

@aivanov-jdk I was waiting on testing results to make the final update along with the above change.

mrserb · 2022-10-07T02:07:28Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

+         * @param d number to be rounded
+         * @return the rounded value
+         */
+        private static int roundHalfDown(double d) {


The sun.java2d.pipe.Region#clipRound() seems have the same purpose, but it also clip the double to int.

The sun.java2d.pipe.Region#clipRound() seems have the same purpose, but it also clip the double to int.

Thank you, @mrserb. It's better to re-use the existing implementation. I tested with it, it gives the same result.

@mrserb Thank you for the suggestion.

I guess roundHalfDown has been replaced with Region.clipRound, so it needs to be removed.

honkar-jdk · 2022-10-07T17:23:46Z

Two edge cases issues were identified while testing with different frame sizes and on different platforms-

One was due to the inherent issue of robot.getPixelColor() which sometimes returns a different pixel color at the edge of border. This is a test issue and is fixed by adding a offset of 1 pixel to the border and corner checks.
The other issue occurs only in specific scenario - when the internal frame size is set to 125, 150, 225, 250...etc on uiScale of 1.25 (on Windows). A thin red line is visible at the bottom of the border. This is might be due to the loopCount value.

…bulk of border

honkar-jdk · 2022-10-07T18:05:09Z

With the current update, on some scales the highlight and shadow lines are not positioned exactly at the middle of the border. This is being investigated currently.

mrserb · 2022-10-07T21:34:35Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

+                width = Region.clipRound(at.getScaleX() * w);
+                height = Region.clipRound(at.getScaleY() * h);
+                xtranslation = Region.clipRound(at.getScaleX() * x + at.getTranslateX());
+                ytranslation = Region.clipRound(at.getScaleY() * y + at.getTranslateY());


This probably should be refactored to something like this:

jdk/src/java.desktop/share/classes/sun/java2d/SunGraphics2D.java

Line 375 in ec4fb47

w = Region.dimAdd(x, Region.clipScale(w, scaleX));

@mrserb Thank for the suggestion. Are you suggesting that same transformations be applied for width, height and additionally to x&y translations?

jdk/src/java.desktop/share/classes/sun/java2d/SunGraphics2D.java

Line 373 in ec4fb47

x = constrainX = (int) transform.getTranslateX();

The methods like Rigion.dimAdd/clipScale should be used everywhere the overflow is possible.

Thank you, @mrserb, for your suggestion. I didn't know about those methods.

We already have JDK-8294680 to refactor this code. If you don't object, I'd rather postpone updating the calculations to a later time. We should absolutely re-use the existing code if it does the same thing.

Thank you, @mrserb, for your suggestion. I didn't know about those methods.

We already have JDK-8294680 to refactor this code. If you don't object, I'd rather postpone updating the calculations to a later time. We should absolutely re-use the existing code if it does the same thing.

@aivanov-jdk I think we decided to retain roundHalfDown and make the changes when refactoring the code.

Thank you, @mrserb, for your suggestion. I didn't know about those methods.
We already have JDK-8294680 to refactor this code. If you don't object, I'd rather postpone updating the calculations to a later time. We should absolutely re-use the existing code if it does the same thing.

@aivanov-jdk I think we decided to retain roundHalfDown and make the changes when refactoring the code.

The comment above is about Rigion.dimAdd and Rigion.clipScale.

As for roundHalfDown, you had replaced it with Region.clipRound. When I was reading the code, I got the impression that you used Region.clipRound everywhere but one place. Eventually, I see you've reverted that change, and you're using roundHalfDown with no trace of clipRound left to be found.

What I saw must have been coming from code comments rather than the real code.

The comment above confirms that using Region.clipRound gives the same result:

The sun.java2d.pipe.Region#clipRound() seems have the same purpose, but it also clip the double to int.

… It's better to re-use the existing implementation. I tested with it, it gives the same result.

It went into #10681.

I just missed that you had reverted it back.

I guess it's not worth re-doing it once again. We'll handle it in JDK-8294680 too.

Sounds good. I'll have it updated during JDK-8294680 changes.

mrserb · 2022-10-07T21:42:40Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

-              g.drawLine( 0, 1, 0, h-2);
-              g.drawLine( w-1, 1, w-1, h-2);
-              g.drawLine( 1, h-1, w-2, h-1);
+            Graphics2D g2d = (Graphics2D) g;


It could be a DebugGraphics which is not a Graphics2D

If the code will be refactored please double-check that the blind cast will not be used, "g" could DebugGraphics which is not a Graphics2D

Thank you for catching it. Made a note of it , to add it during refactoring changes - JDK-8294680

mrserb · 2022-10-07T21:43:44Z

src/java.desktop/share/classes/javax/swing/plaf/metal/MetalBorders.java

+            // skip resetting the transform
+            resetTransform = (at.getShearX() == 0) && (at.getShearY() == 0);
+
+            if (resetTransform) {


Please clarify why we should reset the current transform? How it will work if the user sets a transform and render this to the BufferedImage.

On fractional scaling, painting of lines and borders have issues (lines seem to be separated or look uneven) as shown above, #10274 (comment). In order to paint them uniformly and next to each other, the idea is to render at 1x scaling, apply the required thickness/scaling to the lines at this point (when transform is reset) and then the original (old) transform is restored at the end.

This idea was referenced from a similar issue #7449 (comment).

honkar-jdk · 2022-10-21T22:37:13Z

While testing, following issues were observed -

Uneven border thickness (no. of pixels painted differed on 4 sides) thus making it difficult to evenly place the shadow and highlight lines - This was resolved by using an even multiplier (4) for border thickness instead of odd (5) and changing border insets to same number (4).
Edges of border being painted over background and titlebar resolved by using even multiplier
Shadow and highlight line painted at different offsets at left and bottom of the border even though the start location was same - Adding scaled x, y and respective translate to the scaled width and height as suggested by @aivanov-jdk helped to achieve consistent placement of the lines on all scales.

                w = clipRound(at.getScaleX() * width + xx) - xtranslation;
                h = clipRound(at.getScaleY() * height + yy) - ytranslation;

honkar-jdk · 2022-10-31T16:37:09Z

@aivanov-jdk Fix has been updated since last approval. Can you please re-review.

honkar-jdk added 5 commits September 7, 2022 12:14

JIF Metal Border Scaling - Initial changes

e10aa59

expanded imports

767172e

Added test case, minor position changes to shadow lines

e905d8b

updated test case

0968518

added test summary

955265c

openjdk bot added the rfr Pull request is ready for review label Sep 14, 2022

openjdk bot added the client client-libs-dev@openjdk.org label Sep 14, 2022

azuev-java reviewed Sep 19, 2022

View reviewed changes

Merge branch 'master' into JIFBorder_8015739

b02179c

azuev-java reviewed Sep 20, 2022

View reviewed changes

added local variable for scaled corner

ced69a3

stanio reviewed Sep 21, 2022

View reviewed changes

azuev-java reviewed Sep 29, 2022

View reviewed changes

removed redundant jtreg header

4047f9d

azuev-java approved these changes Sep 30, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 30, 2022

aivanov-jdk reviewed Oct 3, 2022

View reviewed changes

alisenchung suggested changes Oct 3, 2022

View reviewed changes

review changes, saving a scaled version of image

6efd841

aivanov-jdk suggested changes Oct 4, 2022

View reviewed changes

openjdk bot added the rfr Pull request is ready for review label Oct 5, 2022

test updated

2e3b7af

aivanov-jdk reviewed Oct 5, 2022

View reviewed changes

mrserb reviewed Oct 7, 2022

View reviewed changes

fix for pixel color at edge, outermost border lines drawn as part of …

da54767

…bulk of border

mrserb reviewed Oct 7, 2022

View reviewed changes

honkar-jdk added 2 commits October 20, 2022 15:50

highlight and shadow line positioning changes

99664b3

new x,y and translate component added to fix pixel painting issue

4fa3490

		// border and corner scaling
		corner = (int) Math.round(corner * at.getScaleX());

	for (int i = 0; i < 5; i++) {
	for (int i = 0; i < BORDER_THICKNESS; i++) {

JDK-8015739: Background of JInternalFrame is located out of JInternalFrame #10274

JDK-8015739: Background of JInternalFrame is located out of JInternalFrame #10274

Conversation

honkar-jdk commented Sep 14, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Sep 14, 2022

openjdk bot commented Sep 14, 2022

mlbridge bot commented Sep 14, 2022 • edited Loading

Webrevs

Choose a reason for hiding this comment

honkar-jdk Sep 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honkar-jdk Sep 21, 2022 • edited Loading

Choose a reason for hiding this comment

honkar-jdk Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honkar-jdk Sep 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honkar-jdk Sep 30, 2022 • edited Loading

Choose a reason for hiding this comment

azuev-java left a comment

Choose a reason for hiding this comment

openjdk bot commented Sep 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aivanov-jdk commented Oct 3, 2022

Choose a reason for hiding this comment

honkar-jdk Oct 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honkar-jdk Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honkar-jdk Oct 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honkar-jdk commented Oct 3, 2022

honkar-jdk commented Oct 4, 2022 • edited Loading

aivanov-jdk commented Oct 4, 2022

aivanov-jdk commented Oct 4, 2022

aivanov-jdk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honkar-jdk commented Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honkar-jdk commented Oct 7, 2022 • edited Loading

honkar-jdk commented Oct 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honkar-jdk commented Sep 14, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Sep 14, 2022 •

edited

Loading

honkar-jdk Sep 19, 2022 •

edited

Loading

honkar-jdk Sep 21, 2022 •

edited

Loading

honkar-jdk Sep 27, 2022 •

edited

Loading

honkar-jdk Sep 21, 2022 •

edited

Loading

honkar-jdk Sep 30, 2022 •

edited

Loading

openjdk bot commented Sep 30, 2022 •

edited

Loading

honkar-jdk Oct 3, 2022 •

edited

Loading

honkar-jdk Oct 5, 2022 •

edited

Loading

honkar-jdk Oct 3, 2022 •

edited

Loading

honkar-jdk commented Oct 4, 2022 •

edited

Loading

honkar-jdk commented Oct 5, 2022 •

edited

Loading

honkar-jdk commented Oct 7, 2022 •

edited

Loading

honkar-jdk commented Oct 21, 2022 •

edited

Loading

honkar-jdk commented Oct 31, 2022 •

edited

Loading