Buffer Locked but not Unlocked by Model::Clone()

[SirNate0]

When a Model is cloned and the vertex/index buffers are not shadowed, the function attempts to lock the buffer and copy the data that way. Let's ignore for now that this violates the documentation of the Lock() functions that it is for write-only (/// Lock the buffer for write-only editing. Return data pointer if successful. Optionally discard data outside the range.). The method never Unlock()s the buffers, which could cause problems down the line, I imagine (I've not tested this).

https://github.com/urho3d/Urho3D/blob/900611ceeb05e20cedef707bcb6a2e4ac48fb4e1/Source/Urho3D/Graphics/Model.cpp#L645-L651

https://github.com/urho3d/Urho3D/blob/900611ceeb05e20cedef707bcb6a2e4ac48fb4e1/Source/Urho3D/Graphics/Model.cpp#L673-L679

[eugeneko]

There's no reason to use Lock instead of GetShadowData, Lock won't be able to read GPU buffer anyway.

[SirNate0]

Is that a "will never work" or a "generally won't work but with certain drivers may work"? If the former, we should probably just remove the else branches and just log the error.

[eugeneko]

In OpenGL Urho3D there is literally no GPU memory access, upload call is performed Unlock. In DX11, there is write-only GPU access on Lock. In DX9 there may or may not be read GPU access on Lock but it doesn't matter because screw DX9.

In general, Lock can reliably read buffer data only if buffer is shadowed, because it just returns the shadow in this case. So, there's no reason to ever use Lock for reading, because you have GetShadowData for returning shadow.

And even if Lock can actually read GPU data on DX9, it is completely irrelevant.

"generally won't work but with certain drivers may work"

It may be the opposite, btw. "Generally won't work, will crash your Driver/OS with certain drivers"

[github-actions[bot]]

Marking this stale since there has been no activity for 30 days. It will be closed if there is no activity for another 15 days.