Skip to content

Conversation

reapazor
Copy link
Contributor

@reapazor reapazor commented Oct 7, 2021

Purpose of this PR

https://fogbugz.unity3d.com/f/cases/1372213/

Depending on the DPI of the monitor, and the Windows scaling value (125% is the most common trigger), situations arise where the border/title system around VFX graph blocks constantly dirties itself causing a recursive error from the layout engine. This constant spam degrades the performance of the editor.

This change adds specific checks to resolve scenarios where the internal values have been rounded, but the setting values have not. It also adds protection on the event-based call to do a similar check against the old/new values to avoid additional computation.


Testing status

Tested on a 4k screen with DPI set to 125%

@reapazor reapazor self-assigned this Oct 7, 2021
@github-actions github-actions bot added the vfx label Oct 7, 2021
@github-actions
Copy link

github-actions bot commented Oct 7, 2021

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

VFX
/jobDefinition/.yamato%252Fall-vfx.yml%2523PR_VFX_trunk

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

@github-actions
Copy link

github-actions bot commented Oct 7, 2021

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page).
See the PR template for more information.
Thank you!

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to the Unity SRP repo!
Please make sure to fill out the PR template as best you can to give reviewers as much information as possible.
If you have any questions (and you are a Unity employee) go to "#devs-renderpipe"

@reapazor reapazor marked this pull request as draft October 7, 2021 13:34
@reapazor reapazor marked this pull request as ready for review October 7, 2021 20:14
julienamsellem and others added 2 commits October 8, 2021 11:54
Depending on DPI setting the bounds rect could be instable and vary of less than a pixel (especially visible with DPI set at 125%)
@reapazor reapazor merged commit 128eb6b into master Oct 12, 2021
@reapazor reapazor deleted the xp/vfx-systemborder-dpi-fix branch October 12, 2021 14:00
@julienamsellem julienamsellem requested a review from a team October 12, 2021 14:09
Copy link
Contributor

@VladNeykov VladNeykov 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, verified the fix. No issues related to the PR found, tested a few different effects (from simple to one containing all nodes/blocks/contexts) at 100%, 125% and 175%.

Found some cases in which the issue the PR fixes still reproduces, but they are very very edge case and hard to repro..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants