MixedReality-GraphicsTools-Unity icon indicating copy to clipboard operation
MixedReality-GraphicsTools-Unity copied to clipboard

License issue with DrawFullScreenPass.cs

Open shaynie opened this issue 2 years ago • 9 comments

The DrawFullScreenPass.cs file https://github.com/microsoft/MixedReality-GraphicsTools-Unity/blob/v0.5.14/com.microsoft.mrtk.graphicstools.unity/Runtime/Utilities/DrawFullscreenPass.cs contains this comment:

/// Forked from: https://github.com/Unity-Technologies/UniversalRenderingExamples/tree/master/Assets/Scripts/Runtime/RenderPasses

But doesn't copy the license from the repo. The header should show that it is sublicensed from there.

shaynie avatar Nov 09 '23 17:11 shaynie

Here is the license from the root of that repo: https://github.com/Unity-Technologies/UniversalRenderingExamples/blob/master/Assets/License.pdf

shaynie avatar Nov 09 '23 17:11 shaynie

Oh great catch @shaynie! Do you have an example of how the header should reference the sublicense.

Also, the latest version of DrawFullScreenPass has deviated quite a bit from the original to support Unity 2022. So, I wonder if that should be called out?

Cameron-Micka avatar Nov 30 '23 01:11 Cameron-Micka

@Cameron-Micka do you have an ETA on addressing this?

AMollis avatar Feb 14 '24 21:02 AMollis

Once this question is answered I'm happy to fix asap:

Oh great catch @shaynie! Do you have an example of how the header should reference the sublicense.

Cameron-Micka avatar Feb 15 '24 23:02 Cameron-Micka

Hi @Cameron-Micka, do you think there is an alternative DrawFuillscreenPass file that could form a basis for this file that doesn't have the potential license issue of https://github.com/Unity-Technologies/UniversalRenderingExamples/blob/master/Assets/License.pdf (non-commercial only). Or could we clean-room implement it?

ghazen-ml avatar Feb 28 '24 21:02 ghazen-ml

Good question @ghazen-ml, if you look at the current state of the file it has diverged quite a bit from its roots to support XR and Unity 2022+: https://github.com/microsoft/MixedReality-GraphicsTools-Unity/blob/main/com.microsoft.mrtk.graphicstools.unity/Runtime/Utilities/DrawFullscreenPass.cs

In fact, the 2022+ version of the class is a rewrite since Unity changed much of the interface for doing blits. But I'm not sure when we can make the call as something is "clean room" or not. @AMollis do you have any thoughts?

Cameron-Micka avatar Feb 29 '24 16:02 Cameron-Micka

Related Issue:

  • https://github.com/Unity-Technologies/UniversalRenderingExamples/issues/50

AMollis avatar Feb 29 '24 17:02 AMollis

Hi @Cameron-Micka, a few of ideas that came up in the MRTK maintainers group and I have one too:

  1. The DrawFullscreenPass.cs file could be moved into a separate repository or branch so that the the encumbered file could be optional/experimental and left out for mainline usage in MRTK.
  2. A more modern example could be found to base off of, without the encumbered licensed file, or a clean implementation using the unity docs, e.g. https://docs.unity3d.com/Packages/[email protected]/manual/post-processing/post-processing-custom-effect-low-code.html
  3. The file could be removed from the repo
  4. We could prod unity team about https://github.com/Unity-Technologies/UniversalRenderingExamples/issues/50 again (I've started movement on this from my side)

ghazen-ml avatar Jun 15 '24 03:06 ghazen-ml

Thanks for the ideas @ghazen-ml! Thanks for starting the fourth bullet point - if we hear back from Unity that would obviously be the easiest solution. Else, I'm open do doing a clean room implementation since it's not that much to rewrite.

Cameron-Micka avatar Jun 17 '24 22:06 Cameron-Micka