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 editor constant redraw from fxaa and debanding. #43272

Merged
merged 1 commit into from
Nov 2, 2020

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Nov 2, 2020

Every NOTIFICATION_PROCESS the spatial_editor_plugin.cpp is calling set_use_fxaa which is causing a redraw_request(). Same with debanding.

These can be fixed be checking for noop state changes.

Fixes #43228

Notes

  • This will also presumably need matching PR for 4.0, I can't do this (need to upgrade my OS before 4.0 will run) but it should be simple enough for Akien etc.
  • Need to check at startup this matches the default state of fxaa and debanding - yes it does seem to match on client side and in the renderer.

Further

Although this fixes the problem, it strikes me that the whole system of querying a bunch of strings every process in spatial_editor_plugin.cpp (and elsewhere presumably) is very inefficient. Surely there should be a notification sent specifically when a project setting is changed, rather than checking every setting, every frame, just in case? Does anyone know if one exists already? Maybe we could put one in.

This would be very useful in the renderer too to prevent the need for restarts when changing settings.

Every NOTIFICATION_PROCESS the spatial_editor_plugin.cpp is calling set_use_fxaa which is causing a redraw_request(). Same with debanding.

These can be fixed be checking for noop state changes.
@akien-mga
Copy link
Member

Although this fixes the problem, it strikes me that the whole system of querying a bunch of strings every process in spatial_editor_plugin.cpp (and elsewhere presumably) is very inefficient. Surely there should be a notification sent specifically when a project setting is changed, rather than checking every setting, every frame, just in case? Does anyone know if one exists already? Maybe we could put one in.

I agree that the current system feels quite inefficient.
There is NOTIFICATION_EDITOR_SETTINGS_CHANGED but I don't know of any equivalent for ProjectSettings.

@akien-mga akien-mga added this to the 3.2 milestone Nov 2, 2020
@akien-mga akien-mga merged commit df6d845 into godotengine:3.2 Nov 2, 2020
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

This will also presumably need matching PR for 4.0, I can't do this (need to upgrade my OS before 4.0 will run) but it should be simple enough for Akien etc.

Strangely enough the code is already good for both screen space AA and debanding in the master branch, these issues were added when backporting to 3.2.

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.

3 participants