Arvtesh / UnityFx.Outline

Screen-space outlines for Unity3d.
MIT License
1.26k stars 90 forks source link

IgnoreLayers off by 1 error. #71

Open KonstantinRudolph opened 1 year ago

KonstantinRudolph commented 1 year ago

Ignore the following see my second comment on the issue for the real problem:

~~The ignore layermask is off by one in OutlineRendererCollection.cs in internal void Reset(bool includeInactive, int ignoreLayerMask)~~

when the gameObject-layer is shifted it's shifted 1 bit to much due to the shifted bit already starting at the first non-default layer (0)

Built-Ins: Default: 0000 TransparentFX: 0001 IgnoreRaycast: 0010

Example Ignore-Layermask: 1110

gameObject on layer 0001: gameObject.layer == 1 current shiftcode: ((1 << renderer.gameObject.layer) & ignoreLayerMask) == 0) => 0001 << 1 produces 0010 expected is 0001 => the condition fails although the gameobject is on the specified layer: ~~ => 0010 & 1110 == 0010 != 0000~~

correct would be: ((1 << (renderer.gameObject.layer -1)) & ignoreLayerMask) == 0) => 0001 << (1 - 1) produces 0001 expected is 0001 => the condition succeeds for the same layer: => 0001 & 1110 == 0000 == 0000

Of course layer 0 must be handled differently (simply use 0 instead of shifting or limit the shift to be at least 0) to prevent: 1 << -1 (which equals 1 << 31 in c# for int), but that case is blocked by the use all if == 0 anyway

KonstantinRudolph commented 1 year ago

Probably not the most efficient solution but works as a hotfix for me for now: if((1 << Mathf.Max(renderer.gameObject.layer-1, 0) & ignoreLayerMask) == 0) { //[...] }

KonstantinRudolph commented 1 year ago

The real off by 1 error does NOT happen in the collection, but in the custom-editor of the OutlineBehaviour (probably also in the OutlineEffect for the camera but I didn't used this yet):

The problem is the EditorGUI.LayerMask(...) usage var mask = EditorGUILayout.MaskField("Ignore layers", _effect.IgnoreLayerMask, InternalEditorUtility.layers); InternalEditorUtility.layers provides a "crunched" array of masks, leaving out the unused layers For example: Default : 000000 TransparentFx : 000001 IgnoreRaycast: 000010

Water; 001000 UI: 010000 CustomLayer: 100000

But the layers property returns: layers[] { "Default", "TransparentFx", "IgnoreRaycast", "Water", "UI", "CustomLayer" }

When the drawer is used now and the Water layer is set to ignore, instead the bit for the empty layer (bit >> 1) is set. If more empty layers occur setting a flag will set bit >> n where n is the count of empty-layers before the chosen layer.

Since Unity still keeps EditorGUI.LayerMaskField internal (the one which they use to handle the layers correctly more easily) you have to use the two helpers of InternalEditorUtility (what a misnomer for sth not internal.... :) )

int mask = InternalEditorUtility.LayerMaskToConcatenatedLayersMask(_effect.IgnoreLayerMask);
mask = EditorGUILayout.MaskField(mask, InternalEditorUtility.layers);
mask = InternalEditorUtility.ConcatenatedLayersMaskToLayerMask(mask);

or chained:

int mask = InternalEditorUtility.ConcatenatedLayersMaskToLayerMask(
    EditorGUILayout.MaskField(
        InternalEditorUtility.LayerMaskToConcatenatedLayersMask(_effect.IgnoreLayerMask), InternalEditorUtility.layers));
bsoglin commented 3 months ago

Your solution here fixed the problem for me!