Skip to content
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

License issue with DrawFullScreenPass.cs #198

Open
shaynie opened this issue Nov 9, 2023 · 9 comments
Open

License issue with DrawFullScreenPass.cs #198

shaynie opened this issue Nov 9, 2023 · 9 comments
Labels
documentation Improvements or additions to documentation

Comments

@shaynie
Copy link
Contributor

shaynie commented Nov 9, 2023

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
Copy link
Contributor Author

shaynie commented Nov 9, 2023

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

@Cameron-Micka
Copy link
Member

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 Cameron-Micka added the documentation Improvements or additions to documentation label Nov 30, 2023
@AMollis
Copy link
Member

AMollis commented Feb 14, 2024

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

@Cameron-Micka
Copy link
Member

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
Copy link

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
Copy link
Member

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
Copy link
Member

AMollis commented Feb 29, 2024

@ghazen-ml
Copy link

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 License only allows personal and non-commercial use Unity-Technologies/UniversalRenderingExamples#50 again (I've started movement on this from my side)

@Cameron-Micka
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

4 participants