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

Avoid redraw animation on StructuredViewer #927

Merged
merged 1 commit into from
Jul 11, 2023
Merged

Conversation

vogella
Copy link
Contributor

@vogella vogella commented Jul 10, 2023

When the a table contains a big amount of markers, selecting another resolution leads to the typical "animated removal" of all contained items on Windows, which fully blocks the UI thread due to redrawing after each item removed.

This animation is internal behavior of the viewers and seems something which does note help the user due to screen artifacts which are created and slows down the update process.

See also #909

Copy link
Contributor

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

Looks good!

@github-actions
Copy link
Contributor

github-actions bot commented Jul 10, 2023

Test Results

     852 files  ±0       852 suites  ±0   1h 11m 50s ⏱️ + 4m 6s
  7 202 tests ±0    7 053 ✔️ +1  149 💤 ±0  0  - 1 
22 752 runs  ±0  22 265 ✔️ +1  487 💤 ±0  0  - 1 

Results for commit 3fd07d4. ± Comparison against base commit 37284cf.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@Bananeweizen Bananeweizen left a comment

Choose a reason for hiding this comment

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

Since the setRedraw mechanism uses a counter internally, this should be completely fine for every client.

@BeckerWdf
Copy link
Contributor

When the a table contains a big amount of markers,

You mean "entries" not "markers", don't you?

@Bananeweizen
Copy link
Contributor

You mean "entries" not "markers", don't you?

The complete sentence was copied directly from the other issue.

And yes, the general problem is that there is a refresh after every item removed from the viewer, which becomes very visible when going from a huge list of items to no items at all. This happens/happened in the egit file list, the installed plugins list in about, the launch config selected plugins list, and so on...

When the a table contains a big amount of entries, selecting
another resolution leads to the typical "animated removal" of all
contained items on Windows, which fully blocks the UI thread due to
redrawing after each item removed.

This animation is internal behavior of the viewers and seems something
which does note help the user due to screen artifacts which are created
and slows down the update process.

See also #909
@vogella vogella merged commit 3b9c02c into master Jul 11, 2023
@vogella vogella deleted the viewer-refresh branch July 11, 2023 09:31
@vogella
Copy link
Contributor Author

vogella commented Jul 11, 2023

Thanks @Bananeweizen for bring this to our attention, thanks to @laeubi for the feedback to solve this problem directly in JFace and thanks to @BeckerWdf for the review of the commit message.

I will be off my machine for the next 3-4 weeks so if something is wrong with this patch, I will not be able to participate in the discussion.

@jonahgraham
Copy link
Contributor

The code changed in this PR may be called from the non-UI thread so the setRedraw call now introduces a SWTException: Invalid thread access - see eclipse-platform/eclipse.platform#710

@laeubi
Copy link
Contributor

laeubi commented Sep 21, 2023

@jonahgraham I don't think it strictly introduces one as Viewer#setInput must be called from the UI thread and if implementations do differently they need to handle the case there.

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.

5 participants