microsoft / MixedReality-GraphicsTools-Unity

Graphics tools and components for developing Mixed Reality applications in Unity.
MIT License
175 stars 38 forks source link

License issue with DrawFullScreenPass.cs #198

Open shaynie opened 8 months ago

shaynie commented 8 months ago

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 commented 8 months ago

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

Cameron-Micka commented 7 months ago

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?

AMollis commented 4 months ago

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

Cameron-Micka commented 4 months ago

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.

ghazen-ml commented 4 months ago

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?

Cameron-Micka commented 4 months ago

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?

AMollis commented 4 months ago

Related Issue:

ghazen-ml commented 3 weeks ago

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/com.unity.render-pipelines.universal@14.0/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)
Cameron-Micka commented 2 weeks ago

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.