Skip to content

Commit

Permalink
Merge branch 'master' into feature/fix-stackoverflow-in-resolve-lookups
Browse files Browse the repository at this point in the history
  • Loading branch information
@hjohn
hjohn authored Aug 2, 2024
2 parents 404dafb + 686a396 commit 72d64b0
Show file tree
Hide file tree
Showing 10 changed files with 692 additions and 228 deletions.
12 changes: 9 additions & 3 deletions modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2024, 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
Expand Down Expand Up @@ -934,17 +934,23 @@ public TabHeaderArea() {
// Scrolling the mouse wheel downwards results in the tabs scrolling left (i.e. exposing the right-most tabs)
// Scrolling the mouse wheel upwards results in the tabs scrolling right (i.e. exposing th left-most tabs)
addEventHandler(ScrollEvent.SCROLL, (ScrollEvent e) -> {
double dx = e.getDeltaX();
double dy = e.getDeltaY();

Side side = getSkinnable().getSide();
side = side == null ? Side.TOP : side;
switch (side) {
default:
case TOP:
case BOTTOM:
setScrollOffset(scrollOffset + e.getDeltaY());
// Consider vertical scroll events (dy > dx) from mouse wheel and trackpad,
// and horizontal scroll events from a trackpad (dx > dy)
dx = Math.abs(dy) > Math.abs(dx) ? dy : dx;
setScrollOffset(scrollOffset + dx);
break;
case LEFT:
case RIGHT:
setScrollOffset(scrollOffset - e.getDeltaY());
setScrollOffset(scrollOffset - dy);
break;
}

Expand Down
103 changes: 102 additions & 1 deletion modules/javafx.controls/src/test/java/test/javafx/scene/control/TabPaneTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2024, 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
Expand Down Expand Up @@ -38,6 +38,7 @@
import static org.junit.Assert.fail;

import javafx.scene.control.SelectionModel;
import javafx.scene.input.ScrollEvent;
import test.com.sun.javafx.scene.control.infrastructure.StageLoader;
import javafx.application.Platform;
import javafx.beans.property.BooleanProperty;
Expand Down Expand Up @@ -1241,4 +1242,104 @@ public void testSMLeakOnSwitchSkinAndSM() {
attemptGC(10, weakSMRef);
assertNull(weakSMRef.get());
}

@Test
public void testVerticalScrollTopSide() {
scrollTabPane(Side.TOP, 0, -100);
}

@Test
public void testVerticalScrollRightSide() {
scrollTabPane(Side.RIGHT, 0, 100);
}

@Test
public void testVerticalScrollBottomSide() {
scrollTabPane(Side.BOTTOM, 0, -100);
}

@Test
public void testVerticalScrollLeftSide() {
scrollTabPane(Side.LEFT, 0, 100);
}

@Test
public void testHorizontalScrollTopSide() {
scrollTabPane(Side.TOP, -100, 0);
}

@Test
public void testHorizontalScrollRightSide() {
scrollTabPane(Side.RIGHT, 100, 0);
}

@Test
public void testHorizontalScrollBottomSide() {
scrollTabPane(Side.BOTTOM, -100, 0);
}

@Test
public void testHorizontalScrollLeftSide() {
scrollTabPane(Side.LEFT, 100, 0);
}

private void scrollTabPane(Side side, double deltaX, double deltaY) {
tabPane.setMaxSize(400, 100);
tabPane.setSide(side);
for (int i = 0; i < 40; i++) {
Tab tab = new Tab("Tab " + (1000 + i));
tabPane.getTabs().add(tab);
}
root.getChildren().add(tabPane);
stage.show();

Bounds firstTabBounds = tabPane.lookupAll(".tab-label")
.stream()
.findFirst()
.map(n -> n.localToScene(n.getLayoutBounds()))
.orElse(null);
assertNotNull(firstTabBounds);

Bounds layoutBounds = tabPane.getLayoutBounds();
double minX = tabPane.localToScene(layoutBounds).getMinX();
double minY = tabPane.localToScene(layoutBounds).getMinY();
double minScrX = tabPane.localToScreen(layoutBounds).getMinX();
double minScrY = tabPane.localToScreen(layoutBounds).getMinY();
double x = 50;
double y = 10;

SceneHelper.processMouseEvent(scene,
MouseEventGenerator.generateMouseEvent(MouseEvent.MOUSE_MOVED, minX + x, minY + y));
tk.firePulse();

StackPane tabHeaderArea = (StackPane) tabPane.lookup(".tab-header-area");
assertNotNull(tabHeaderArea);

Event.fireEvent(tabHeaderArea, new ScrollEvent(
ScrollEvent.SCROLL,
minX + x, minY + y,
minScrX + x, minScrY + y,
false, false, false, false, true, false,
deltaX, deltaY, deltaX, deltaY,
ScrollEvent.HorizontalTextScrollUnits.NONE, 0.0,
ScrollEvent.VerticalTextScrollUnits.NONE, 0.0,
0, null));
tk.firePulse();

Bounds newFirstTabBounds = tabPane.lookupAll(".tab-label")
.stream()
.findFirst()
.map(n -> n.localToScene(n.getLayoutBounds()))
.orElse(null);
assertNotNull(newFirstTabBounds);

if (side.equals(Side.TOP) || side.equals(Side.BOTTOM)) {
double delta = Math.abs(deltaY) > Math.abs(deltaX) ? deltaY : deltaX;
assertEquals(firstTabBounds.getMinX() + delta, newFirstTabBounds.getMinX(), 0);
assertEquals(firstTabBounds.getMinY(), newFirstTabBounds.getMinY(), 0);
} else {
assertEquals(firstTabBounds.getMinX(), newFirstTabBounds.getMinX(), 0);
assertEquals(firstTabBounds.getMinY() - deltaY, newFirstTabBounds.getMinY(), 0);
}
}
}
87 changes: 58 additions & 29 deletions modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,15 @@
import com.sun.glass.ui.Pixels;
import com.sun.javafx.tk.Toolkit;

import java.util.HashMap;
import java.util.Map;
import java.util.List;

import javafx.collections.ListChangeListener;
import javafx.collections.ObservableList;
import javafx.beans.InvalidationListener;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.collections.transformation.FilteredList;
import javafx.scene.image.Image;
import javafx.scene.image.ImageView;
Expand All @@ -57,6 +61,9 @@ class GlassSystemMenu implements TKSystemMenu {

private List<MenuBase> systemMenus = null;
private MenuBar glassSystemMenuBar = null;
private final Map<Menu, ListChangeListener<MenuItemBase>> menuListeners = new HashMap<>();
private final Map<ListChangeListener<MenuItemBase>, ObservableList<MenuItemBase>> listenerItems = new HashMap<>();
private BooleanProperty active;

private InvalidationListener visibilityListener = valueModel -> {
if (systemMenus != null) {
Expand Down Expand Up @@ -85,6 +92,10 @@ protected MenuBar getMenuBar() {
}

@Override public void setMenus(List<MenuBase> menus) {
if (active != null) {
active.set(false);
}
active = new SimpleBooleanProperty(true);
systemMenus = menus;
if (glassSystemMenuBar != null) {

Expand All @@ -111,11 +122,19 @@ protected MenuBar getMenuBar() {

// Clear the menu to prevent a memory leak, as outlined in RT-34779
private void clearMenu(Menu menu) {
ListChangeListener<MenuItemBase> lcl = menuListeners.get(menu);
if (lcl != null) {
ObservableList<MenuItemBase> target = listenerItems.get(lcl);
target.removeListener(lcl);
menuListeners.remove(menu);
listenerItems.remove(lcl);
}

for (int i = menu.getItems().size() - 1; i >= 0; i--) {
Object o = menu.getItems().get(i);

if (o instanceof MenuItem) {
((MenuItem)o).setCallback(null);
menu.remove(i);
} else if (o instanceof Menu) {
clearMenu((Menu) o);
}
Expand Down Expand Up @@ -148,7 +167,33 @@ private void insertMenu(final Menu parent, final MenuBase mb, int pos) {

final FilteredList<MenuItemBase> filteredItems = items.filtered(x -> x.isVisible());

filteredItems.addListener((ListChangeListener.Change<? extends MenuItemBase> change) -> {
ListChangeListener<MenuItemBase> menuItemListener = createListener(glassMenu);
filteredItems.addListener(menuItemListener);
menuListeners.put(glassMenu, menuItemListener);
listenerItems.put(menuItemListener, filteredItems);

for (MenuItemBase item : items) {
if (item instanceof MenuBase baseItem) {
// submenu
addMenu(glassMenu, baseItem);
} else {
// menu item
addMenuItem(glassMenu, item);
}
}
glassMenu.setPixels(getPixels(mb));

setMenuBindings(glassMenu, mb);

if (parent != null) {
parent.insert(glassMenu, pos);
} else {
glassSystemMenuBar.insert(glassMenu, pos);
}
}

private ListChangeListener<MenuItemBase> createListener(final Menu glassMenu) {
return (ListChangeListener.Change<? extends MenuItemBase> change) -> {
while (change.next()) {
int from = change.getFrom();
int to = change.getTo();
Expand All @@ -157,6 +202,8 @@ private void insertMenu(final Menu parent, final MenuBase mb, int pos) {
for (int i = from + removed.size() - 1; i >= from ; i--) {
List<Object> menuItemList = glassMenu.getItems();
if (i >= 0 && menuItemList.size() > i) {
Object item = menuItemList.get(i);
if (item instanceof Menu menu) clearMenu(menu);
glassMenu.remove(i);
}
}
Expand All @@ -169,32 +216,14 @@ private void insertMenu(final Menu parent, final MenuBase mb, int pos) {
}
}
}
});

for (MenuItemBase item : items) {
if (item instanceof MenuBase) {
// submenu
addMenu(glassMenu, (MenuBase)item);
} else {
// menu item
addMenuItem(glassMenu, item);
}
}
glassMenu.setPixels(getPixels(mb));

setMenuBindings(glassMenu, mb);

if (parent != null) {
parent.insert(glassMenu, pos);
} else {
glassSystemMenuBar.insert(glassMenu, pos);
}
};
}

private void setMenuBindings(final Menu glassMenu, final MenuBase mb) {
mb.textProperty().addListener(valueModel -> glassMenu.setTitle(parseText(mb)));
mb.disableProperty().addListener(valueModel -> glassMenu.setEnabled(!mb.isDisable()));
mb.mnemonicParsingProperty().addListener(valueModel -> glassMenu.setTitle(parseText(mb)));

protected void setMenuBindings(final Menu glassMenu, final MenuBase mb) {
mb.textProperty().when(active).subscribe(valueModel -> glassMenu.setTitle(parseText(mb)));
mb.disableProperty().when(active).subscribe(valueModel -> glassMenu.setEnabled(!mb.isDisable()));
mb.mnemonicParsingProperty().when(active).subscribe(valueModel -> glassMenu.setTitle(parseText(mb)));
}

private void addMenuItem(Menu parent, final MenuItemBase menuitem) {
Expand Down Expand Up @@ -256,7 +285,7 @@ private void insertMenuItem(final Menu parent, final MenuItemBase menuitem, int
glassSubMenuItem.setPixels(getPixels(menuitem));
});

glassSubMenuItem.setEnabled(! menuitem.isDisable());
glassSubMenuItem.setEnabled(!menuitem.isDisable());
menuitem.disableProperty().addListener(valueModel -> glassSubMenuItem.setEnabled(!menuitem.isDisable()));

setShortcut(glassSubMenuItem, menuitem);
Expand All @@ -265,11 +294,11 @@ private void insertMenuItem(final Menu parent, final MenuItemBase menuitem, int
menuitem.mnemonicParsingProperty().addListener(valueModel -> glassSubMenuItem.setTitle(parseText(menuitem)));

if (menuitem instanceof CheckMenuItemBase) {
final CheckMenuItemBase checkItem = (CheckMenuItemBase)menuitem;
final CheckMenuItemBase checkItem = (CheckMenuItemBase) menuitem;
glassSubMenuItem.setChecked(checkItem.isSelected());
checkItem.selectedProperty().addListener(valueModel -> glassSubMenuItem.setChecked(checkItem.isSelected()));
} else if (menuitem instanceof RadioMenuItemBase) {
final RadioMenuItemBase radioItem = (RadioMenuItemBase)menuitem;
final RadioMenuItemBase radioItem = (RadioMenuItemBase) menuitem;
glassSubMenuItem.setChecked(radioItem.isSelected());
radioItem.selectedProperty().addListener(valueModel -> glassSubMenuItem.setChecked(radioItem.isSelected()));
}
Expand Down
26 changes: 5 additions & 21 deletions modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -668,8 +668,7 @@ void recalculateRelativeSizeProperties(final Node node, Font fontForRelativeSize
CascadingStyle style = getStyle(node, property, styleMap, transitionStates[0]);
if (style != null) {
final ParsedValue cssValue = style.getParsedValue();
ObjectProperty<StyleOrigin> whence = new SimpleObjectProperty<>(style.getOrigin());
ParsedValue resolved = resolveLookups(node, cssValue, styleMap, transitionStates[0], whence, new HashSet<>());
ParsedValue resolved = resolveLookups(node, cssValue, styleMap, transitionStates[0], new HashSet<>());
boolean isRelative = ParsedValueImpl.containsFontRelativeSize(resolved, false);
if (!isRelative) {
continue;
Expand Down Expand Up @@ -1319,7 +1318,6 @@ private ParsedValue resolveLookups(
final Styleable styleable,
final ParsedValue parsedValue,
final StyleMap styleMap, Set<PseudoClass> states,
final ObjectProperty<StyleOrigin> whence,
Set<ParsedValue> resolves) {

//
Expand Down Expand Up @@ -1348,18 +1346,6 @@ private ParsedValue resolveLookups(
throw new IllegalArgumentException("Loop detected in " + resolved.getRule().toString() + " while resolving '" + sval + "'");
}

// The origin of this parsed value is the greatest of
// any of the resolved reference. If a resolved reference
// comes from an inline style, for example, then the value
// calculated from the resolved lookup should have inline
// as its origin. Otherwise, an inline style could be
// stored in shared cache.
final StyleOrigin wOrigin = whence.get();
final StyleOrigin rOrigin = resolved.getOrigin();
if (rOrigin != null && (wOrigin == null || wOrigin.compareTo(rOrigin) < 0)) {
whence.set(rOrigin);
}

// the resolved value may itself need to be resolved.
// For example, if the value "color" resolves to "base",
// then "base" will need to be resolved as well.
Expand Down Expand Up @@ -1390,7 +1376,7 @@ private ParsedValue resolveLookups(
for (int ll=0; ll<layers[l].length; ll++) {
if (layers[l][ll] == null) continue;
resolved[l][ll] =
resolveLookups(styleable, layers[l][ll], styleMap, states, whence, resolves);
resolveLookups(styleable, layers[l][ll], styleMap, states, resolves);
}
}

Expand All @@ -1404,7 +1390,7 @@ private ParsedValue resolveLookups(
for (int l=0; l<layer.length; l++) {
if (layer[l] == null) continue;
resolved[l] =
resolveLookups(styleable, layer[l], styleMap, states, whence, resolves);
resolveLookups(styleable, layer[l], styleMap, states, resolves);
}

return new ParsedValueImpl(resolved, parsedValue.getConverter(), false);
Expand Down Expand Up @@ -1537,8 +1523,7 @@ private CalculatedValue calculateValue(
ParsedValue resolved = null;
try {

ObjectProperty<StyleOrigin> whence = new SimpleObjectProperty<>(style.getOrigin());
resolved = resolveLookups(styleable, cssValue, styleMap, states, whence, new HashSet<>());
resolved = resolveLookups(styleable, cssValue, styleMap, states, new HashSet<>());

final String property = cssMetaData.getProperty();

Expand Down Expand Up @@ -1630,8 +1615,7 @@ else if (resolved.getConverter() != null)
else
val = cssMetaData.getConverter().convert(resolved, fontForFontRelativeSizes);

final StyleOrigin origin = whence.get();
return new CalculatedValue(val, origin, isRelative);
return new CalculatedValue(val, style.getOrigin(), isRelative);

} catch (ClassCastException cce) {
final String msg = formatUnresolvedLookupMessage(styleable, cssMetaData, style.getStyle(),resolved, cce);
Expand Down
Loading

0 comments on commit 72d64b0

Please sign in to comment.