Skip to content

8345261: Refactor the Dimension2D classes #1653

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

8345261: Refactor the Dimension2D classes #1653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2576c31
Initial commit
nlisker Nov 29, 2024
e1ce604
Revert static import wildcard use
nlisker Nov 29, 2024
2a72347
Address review comments
nlisker Jan 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/screencast/TokenItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,16 @@

package com.sun.glass.ui.gtk.screencast;

import com.sun.javafx.geom.Dimension;
import com.sun.javafx.geom.Rectangle;
import static com.sun.glass.ui.gtk.screencast.ScreencastHelper.SCREENCAST_DEBUG;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static com.sun.glass.ui.gtk.screencast.ScreencastHelper.SCREENCAST_DEBUG;
import com.sun.javafx.geom.Rectangle;
import com.sun.javafx.geom.Rectangle.Dimension2Di;


/**
Expand Down Expand Up @@ -76,15 +77,15 @@ public boolean hasAllScreensWithExactMatch(List<Rectangle> bounds) {
return allowedScreensBounds.containsAll(bounds);
}

public boolean hasAllScreensOfSameSize(List<Dimension> screenSizes) {
public boolean hasAllScreensOfSameSize(List<Dimension2Di> screenSizes) {
// We also need to consider duplicates, since there may be
// multiple screens of the same size.
// The token item must also have at least the same number
// of screens with that size.

List<Dimension> tokenSizes = allowedScreensBounds
List<Dimension2Di> tokenSizes = allowedScreensBounds
.stream()
.map(bounds -> new Dimension(bounds.width, bounds.height))
.map(Dimension2Di::new)
.collect(Collectors.toCollection(ArrayList::new));

return screenSizes.size() == screenSizes
Expand Down
19 changes: 7 additions & 12 deletions modules/javafx.graphics/src/main/java/com/sun/glass/ui/gtk/screencast/TokenStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@

package com.sun.glass.ui.gtk.screencast;

import com.sun.javafx.geom.Dimension;
import com.sun.javafx.geom.Rectangle;
import static com.sun.glass.ui.gtk.screencast.ScreencastHelper.SCREENCAST_DEBUG;
import static java.nio.file.StandardWatchEventKinds.*;

import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.IOException;
Expand All @@ -45,11 +46,8 @@
import java.util.Properties;
import java.util.Set;

import static java.nio.file.StandardWatchEventKinds.ENTRY_CREATE;
import static java.nio.file.StandardWatchEventKinds.ENTRY_DELETE;
import static java.nio.file.StandardWatchEventKinds.ENTRY_MODIFY;
import static java.nio.file.StandardWatchEventKinds.OVERFLOW;
import static com.sun.glass.ui.gtk.screencast.ScreencastHelper.SCREENCAST_DEBUG;
import com.sun.javafx.geom.Rectangle;
import com.sun.javafx.geom.Rectangle.Dimension2Di;

/**
* Helper class for persistent storage of ScreenCast restore tokens
Expand Down Expand Up @@ -365,13 +363,10 @@ static Set<TokenItem> getTokens(List<Rectangle> affectedScreenBounds) {

// 2. Try screens of the same size but in different locations,
// screens may have been moved while the token is still valid
List<Dimension> dimensions =
List<Dimension2Di> dimensions =
affectedScreenBounds
.stream()
.map(rectangle -> new Dimension(
rectangle.width,
rectangle.height
))
.map(Dimension2Di::new)
.toList();

for (TokenItem tokenItem : allTokenItems) {
Expand Down
4 changes: 2 additions & 2 deletions modules/javafx.graphics/src/main/java/com/sun/javafx/geom/Arc2D.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,9 @@ public Point2D getEndPoint() {
* @param closure The closure type for the arc:
* {@link #OPEN}, {@link #CHORD}, or {@link #PIE}.
*/
public void setArc(Point2D loc, Dimension2D size,
public void setArc(Point2D loc, Dimension2Df size,
float angSt, float angExt, int closure) {
setArc(loc.x, loc.y, size.width, size.height, angSt, angExt, closure);
setArc(loc.x, loc.y, size.width(), size.height(), angSt, angExt, closure);
}

/**
Expand Down
52 changes: 0 additions & 52 deletions modules/javafx.graphics/src/main/java/com/sun/javafx/geom/Dimension.java
View file
Open in desktop

This file was deleted.

19 changes: 5 additions & 14 deletions ...java/com/sun/javafx/geom/Dimension2D.java → ...ava/com/sun/javafx/geom/Dimension2Df.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,9 @@
package com.sun.javafx.geom;

/**
* The <code>Dimension2D</code> class is to encapsulate a width
* and a height dimension.
* <p>
* A 2D dimension object that contains a width and a height as floats.
*
* @see com.sun.javafx.geom.Rectangle.Dimension2Di Dimension2Di - ints
* @see javafx.geometry.Dimension2D Dimension2D - doubles
*/
public class Dimension2D {
public float width;
public float height;

public Dimension2D() { }

public Dimension2D(float w, float h) {
width = w;
height = h;
}
}
public record Dimension2Df(float width, float height) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this class really needed?

Looks like it's only referenced from Arc2D::setArc() and RectanglularShape::setFrame(), as far as I can tell these two methods are never called.

Should all three be removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not familiar with these methods, but I did wonder why we need extra implementations in the initial comment.

Copy link
Contributor

@andy-goryachev-oracle andy-goryachev-oracle Jan 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

looks like dead code left over from the original implementation.

I removed the class along with the two methods and jfx builds (you need to merge the latest master though) and runs (i.e. shows arcs and rectangles) just fine. Eclipse IDE also has no issues. I think we can remove these.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't mind removing them in this PR if others agree.

16 changes: 16 additions & 0 deletions modules/javafx.graphics/src/main/java/com/sun/javafx/geom/Rectangle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,4 +685,20 @@ public int hashCode() {
public String toString() {
return getClass().getName() + "[x=" + x + ",y=" + y + ",width=" + width + ",height=" + height + "]";
}

/**
* A 2D dimension object that contains a width and a height as integers.
*
* @see com.sun.javafx.geom.Dimension2Df Dimension2Df - floats
* @see javafx.geometry.Dimension2D Dimension2D - doubles
*/
public static record Dimension2Di(int width, int height) {

/**
* Captures the width and height of a {@code Rectangle}.
*/
public Dimension2Di(Rectangle rect) {
this(rect.width, rect.height);
}
}
}
4 changes: 2 additions & 2 deletions modules/javafx.graphics/src/main/java/com/sun/javafx/geom/RectangularShape.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ public float getCenterY() {
* @param size the specified <code>Dimension2D</code>
* @see #getFrame
*/
public void setFrame(Point2D loc, Dimension2D size) {
setFrame(loc.x, loc.y, size.width, size.height);
public void setFrame(Point2D loc, Dimension2Df size) {
setFrame(loc.x, loc.y, size.width(), size.height());
}

/**
Expand Down
6 changes: 3 additions & 3 deletions modules/javafx.graphics/src/main/java/javafx/geometry/Dimension2D.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@

import javafx.beans.NamedArg;


/**
* A 2D dimension object that contains a width and a height.
*
* @since JavaFX 2.0
*/
public class Dimension2D {

/**
* Constructs a <code>Dimension2D</code> with the specified width and
* height.
Expand All @@ -51,7 +51,7 @@ public Dimension2D(@NamedArg("width") double width, @NamedArg("height") double h
*
* @defaultValue 0.0
*/
private double width;
private final double width;

/**
* The width of the dimension.
Expand All @@ -66,7 +66,7 @@ public final double getWidth() {
*
* @defaultValue 0.0
*/
private double height;
private final double height;

/**
* The height of the dimension.
Expand Down