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

Fix for scrolling content #112

Merged
merged 1 commit into from Sep 11, 2022
Merged

Fix for scrolling content #112

merged 1 commit into from Sep 11, 2022

Conversation

tchudyk
Copy link
Contributor

@tchudyk tchudyk commented Sep 11, 2022

Probably, after commit [https://github.com//pull/106] I have noticed a negative impact on RichTextFx - during typing text in editor it starts "minimally" scrolling.

I see, that in mentioned commit there is Math.round(offset) in VirtualizedScrollPane:

   private void setVPosition(double pos) {
        // ....
        content.estimatedScrollYProperty().setValue((double) Math.round(offset));
    }

But farther, when this action goes to VirtualFlow class - to method:

void setLengthOffset(double pixels) {
        double total = totalLengthEstimateProperty().getOrElse(0.0);
        double length = sizeTracker.getViewportLength();
        double max = Math.max(total - length, 0);
        double current = lengthOffsetEstimate.getValue();

        if(pixels > max) pixels = max;
        if(pixels < 0) pixels = 0;

        double diff = pixels - current;
        if(diff == 0) {
            // do nothing
        } else if(Math.abs(diff) <= length) { // distance less than one screen
            navigator.scrollCurrentPositionBy(diff);
        } else {
            jumpToAbsolutePosition(pixels);
        }
    }

The incoming pixels value is rounded, but value in variable current is not rounded, and when we compute diff:

double diff = pixels - current;

... there is very often computed value 0.5 (but it should be 0) and VirtualFlow moves visible content by this diff (navigator.scrollCurrentPositionBy(diff)) instead of // do nothing

My fix here is to apply rounding on lengthOffsetEstimate. This solves this issue.

@Jugen
Copy link
Contributor

Jugen commented Sep 11, 2022

@tchudyk Thank you very much for your analysis and PR request. It's very much appreciated. Will merge ....

@Jugen Jugen merged commit b5ef653 into FXMisc:master Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants