-
Notifications
You must be signed in to change notification settings - Fork 792
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
Bugfix: 1238833 #283
Bugfix: 1238833 #283
Conversation
@skhiat please add a changelog (add the entry at the end of the Fixed section (not at the beginning :) ) |
Oh boy. This is a new things introduced in recent trunk it seams.
It is possible that decal is the only one using it in OnSceneGUI which can be a long lasting mistake. @remi-chapelain have you tested with multiedition? Multi-target is used to compound operation on each target of the selection generally. So this can broke the multi-edition at some extend. This need a thorough check. |
@@ -150,22 +150,15 @@ public Bounds OnGetFrameBounds() | |||
public void UpdateMaterialEditor() | |||
{ | |||
int validMaterialsCount = 0; | |||
for (int index = 0; index < targets.Length; ++index) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this problematic loop on target.
It has no meanings on this file l.112 as it is used as a hook on each DecalProjector when the material change. So there we should not use the target notion. We should have a DecalProjector parameter to gather changes.
On line 109 though, the same function is called from OnEnable where multiselection can work on several target at a time.
It seams that this function (original version) is OK for line 109 but we should add another dedicated callback for the individual updated used in event
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After slack discussion. The problem seems to come from another direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, thanks for pointing that out @RSlysz
It breaks the multi material-editing on decals.
Before the PR, when selecting multiple decal projector with different material, you could edit all materials at once in the inspector window. With this fix you can only edit one.
Replaced by PR #413 |
* Fix incorrect condition for GPUEvent Mistake introduced at #4457 * *Add some editorTest * Add some test to covers corners case See https://unity.slack.com/archives/G1BTWN88Z/p1632919330247400 * *WIP* Improve incorrect connection handling * Fix multiple type in output of initialize part * Fix Update to Update which is actually allowed * *Update changelog.md * Fix missing case for VFX.Update tyep * Rename confusing "primaryType"
Owner changed: @RSlysz
Bug:
https://fogbugz.unity3d.com/f/cases/1238833/
To repro: enable guizmo (check the video).
Code:
Apparently in this context it's forbidden to use multicontext (message-error).
Yamato: (Select your branch):
https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics
Any test projects to go with this to help reviewers?
Comments to reviewers
Notes for the reviewers you have assigned.