Skip to content

Commit

Permalink
fix: Horizontal scrollbar flicker when scale is 1.0 and container shr…
Browse files Browse the repository at this point in the history 
…inking

The HSB was shown then hidden quickly creating flicker when the
container is shrank. The new conde disables the scrollbar when scale
factor is 1.0, and to AS_NEEDED whe  scale factor differs.
  • Loading branch information
@bric3
bric3 committed Mar 24, 2024
1 parent fe4aeb9 commit a416731
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 45 deletions.
20 changes: 13 additions & 7 deletions ...lace-swing/src/main/java/io/github/bric3/fireplace/flamegraph/FlamegraphRenderEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.util.stream.Collectors;

/**
* Engine that paint a flamegraph.
* Engine that paints a flamegraph.
*
* <p>
* This controls the global flamegraph rendering, and allow to
Expand Down Expand Up @@ -185,7 +185,6 @@ public FlamegraphRenderEngine(@NotNull FrameRenderer<@NotNull T> frameRenderer)
* @return The height.
*/
public int computeVisibleFlamegraphMinimapHeight(int thumbnailWidth) {
checkReady();
assert thumbnailWidth > 0 : "minimap width must be superior to 0";

// Somewhat it is a best-effort to draw something that shows
Expand Down Expand Up @@ -305,7 +304,6 @@ private void internalPaint(
boolean minimapMode,
boolean icicle
) {
checkReady();
if (frameModel.frames.isEmpty()) {
return;
}
Expand Down Expand Up @@ -515,7 +513,9 @@ public Rectangle getFrameRectangle(
@NotNull Rectangle2D bounds,
@NotNull Point point
) {
checkReady();
if (frameModel.frames.isEmpty()) {
return Optional.empty();
}

int depth = computeFrameDepth(g2, bounds, point);
double xLocation = point.x / bounds.getWidth();
Expand Down Expand Up @@ -572,7 +572,9 @@ public void hoverFrame(
@NotNull Rectangle2D bounds,
@NotNull Consumer<@NotNull Rectangle> hoverConsumer
) {
checkReady();
if (frameModel.frames.isEmpty()) {
return;
}

var oldHoveredFrame = hoveredFrame;
if (frame == oldHoveredFrame) {
Expand All @@ -595,7 +597,9 @@ public void stopHover(
@NotNull Rectangle2D bounds,
@NotNull Consumer<@NotNull Rectangle> hoverConsumer
) {
checkReady();
if (frameModel.frames.isEmpty()) {
return;
}

var oldHoveredFrame = hoveredFrame;
var oldHoveredSiblingFrame = hoveredSiblingFrames;
Expand Down Expand Up @@ -634,7 +638,9 @@ public void stopHover(
Rectangle2D viewRect,
Point point
) {
checkReady();
if (frameModel.frames.isEmpty()) {
return Optional.empty();
}

return getFrameAt(g2, bounds, point).map(frame -> {
// TODO refactor to make frame selection explicit, possibly via toggleSelectedFrameAt
Expand Down
75 changes: 37 additions & 38 deletions fireplace-swing/src/main/java/io/github/bric3/fireplace/flamegraph/FlamegraphView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
* {@link FrameBox}.
* </p>
* <p>
* It can be used is as follows:
* It can be used as follows:
* <pre><code>
* var flamegraphView = new FlamegraphView&lt;MyNode&gt;();
* flamegraphView.setShowMinimap(false);
Expand Down Expand Up @@ -341,30 +341,24 @@ public void layoutContainer(Container parent) {
int oldVpWidth = oldViewPortSize.width;
var vpSize = vp.getSize(oldViewPortSize);

// Never show the horizontal scrollbar when the scale factor is 1.0
// Only change it when necessary
int horizontalScrollBarPolicy = jScrollPane.getHorizontalScrollBarPolicy();
double lastScaleFactor = canvas.zoomModel.getLastScaleFactor();
int newPolicy = lastScaleFactor == 1.0 ?
ScrollPaneConstants.HORIZONTAL_SCROLLBAR_NEVER :
ScrollPaneConstants.HORIZONTAL_SCROLLBAR_AS_NEEDED;
if (horizontalScrollBarPolicy != newPolicy) {
jScrollPane.setHorizontalScrollBarPolicy(newPolicy);
}

// view port has been resized
if (vpSize.width != oldVpWidth) {
// if old fg width == old vp width
// the scaleFactor is 1.0
// => recompute the fg size
// if old fg width > old vp width
// the scaleFactor is > 1.0
// => compute the scaleFactor
// => scale the fg size using the current vp width
// if old fg width < old vp width
// ==> do nothing
int oldFlamegraphWidth = flamegraphSize.width;
if (oldFlamegraphWidth == oldVpWidth) {
canvas.updateFlamegraphDimension(
flamegraphSize,
vpSize.width
);
} else {
// scale the fg size with the new viewport width
canvas.updateFlamegraphDimension(
flamegraphSize,
(int) (((double) vpSize.width) / canvas.zoomModel.getLastScaleFactor())
);
}
// scale the fg size with the new viewport width
canvas.updateFlamegraphDimension(
flamegraphSize,
(int) (((double) vpSize.width) / lastScaleFactor)
);
vp.setViewSize(flamegraphSize);

// if view position X > 0
Expand Down Expand Up @@ -571,7 +565,7 @@ public boolean isShowHoveredSiblings() {
}

/**
* Sets the display mode, either {@link Mode#FLAMEGRAPH} or {@link Mode#ICICLEGRAPH}.
* Sets the display mode, either {@link FlamegraphView.Mode#FLAMEGRAPH} or {@link Mode#ICICLEGRAPH}.
*
* @param mode The display mode.
*/
Expand Down Expand Up @@ -758,7 +752,7 @@ public void requestRepaint() {
canvas.triggerMinimapGeneration();
}

public void overrideZoomAction(@NotNull ZoomAction zoomActionOverride) {
public void overrideZoomAction(@NotNull FlamegraphView.ZoomAction zoomActionOverride) {
Objects.requireNonNull(zoomActionOverride);
this.canvas.zoomActionOverride = zoomActionOverride;
}
Expand All @@ -783,9 +777,9 @@ public void zoomTo(@NotNull FrameBox<@NotNull T> frame) {
* Higher level zoom op that operates on the view and model.
* <p>
* This method will call under the hood {@link FlamegraphCanvas#zoom(ZoomTarget)}.
* Possibly a zoom override ({@link #overrideZoomAction(ZoomAction)} will be set up and may be call instead,
* Possibly a zoom override ({@link #overrideZoomAction(FlamegraphView.ZoomAction)} will be set up and may be call instead,
* but under the hood this will call {@link FlamegraphCanvas#zoom(ZoomTarget)} via it's
* {@link ZoomableComponent#zoom(ZoomTarget)}.
* {@link FlamegraphView.ZoomableComponent#zoom(ZoomTarget)}.
*
* @param canvas the canvas.
* @param zoomTarget the zoom target.
Expand Down Expand Up @@ -914,7 +908,7 @@ public void componentShown(ComponentEvent e) {
// * The guard uses the hash code of the model because the model can be changed,
// and running this listener is necessary to prevent the horizontal scrollbar as well.
if (fgCanvas.flamegraphRenderEngine != null
&& fgCanvas.flamegraphRenderEngine.getFrameModel() != null) {
&& !fgCanvas.flamegraphRenderEngine.getFrameModel().frames.isEmpty()) {
int newHashCode = fgCanvas.flamegraphRenderEngine.getFrameModel().hashCode();
if (newHashCode == frameModelHashCode) {
return;
Expand Down Expand Up @@ -1079,7 +1073,7 @@ private void paintDetails(@NotNull Graphics2D g2) {
}

private void paintMinimap(@NotNull Graphics g, @NotNull Rectangle visibleRect) {
if (flamegraphDimension != null && showMinimap && minimap != null) {
if (showMinimap && minimap != null) {
var g2 = (Graphics2D) g.create(
visibleRect.x + minimapBounds.x,
visibleRect.y + visibleRect.height - minimapBounds.height - minimapBounds.y,
Expand Down Expand Up @@ -1295,10 +1289,6 @@ public void mouseDragged(@NotNull MouseEvent e) {
}

private void processMinimapMouseEvent(@NotNull MouseEvent e) {
if (flamegraphDimension == null) {
return;
}

var pt = e.getPoint();
if (!(e.getComponent() instanceof FlamegraphView.FlamegraphCanvas)) {
return;
Expand Down Expand Up @@ -1329,7 +1319,6 @@ public void mouseMoved(@NotNull MouseEvent e) {
this.addMouseListener(mouseAdapter);
this.addMouseMotionListener(mouseAdapter);

// TODO Record last position after user interaction
new UserPositionRecorderMouseAdapter(this).install(scrollPane);
}

Expand Down Expand Up @@ -1460,7 +1449,7 @@ public void zoom(@NotNull ZoomTarget<@NotNull T> zoomTarget) {
* </p>
*
* @param <T>
* @see HoverListener
* @see FlamegraphView.HoverListener
*/
private static class FlamegraphHoveringScrollPaneMouseListener<T> implements MouseInputListener, FocusListener {
private Point pressedPoint;
Expand Down Expand Up @@ -1713,11 +1702,21 @@ private static class ZoomModel<T> {

/**
* Internal field to track the last user interaction
* of the user that leads to a modification of the position ratio.
* Either the last zoom, the last mouse drag, or the last scroll (trackpad included).
* of the user that leads to a modification of the position ratio on the interval [0.0, 1.0].
* This is also referred to as <em>startX</em>.
* Modified either the last zoom, the last mouse drag, or the last scroll (trackpad included).
*/
private double lastUserInteractionStartX = 0.0;
/**
* Internal field to track the last user interaction
* of the user that leads to a modification of the end position on the interval [0.0, 1.0].
* This is also referred to as <em>endX</em>.
* Modified either the last zoom, the last mouse drag, or the last scroll (trackpad included).
*/
private double lastUserInteractionEndX = 0.0;
/**
* The scale factor resulting from the last user interaction.
*/
private double lastScaleFactor = 1.0;

private final Rectangle canvasVisibleRect = new Rectangle(); // reused
Expand All @@ -1735,7 +1734,7 @@ public void recordLastPositionFromZoomTarget(
} else {
lastUserInteractionStartX = currentZoomTarget.targetFrame.startX;
lastUserInteractionEndX = currentZoomTarget.targetFrame.endX;

canvas.computeVisibleRect(canvasVisibleRect);
this.lastScaleFactor = FlamegraphRenderEngine.getScaleFactor(
canvasVisibleRect.width,
Expand Down

0 comments on commit a416731

Please sign in to comment.