[10.x] Metal support updates [Hold] #1795
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -191,13 +191,6 @@ | |
| }, | ||
| "url": "https://artifactory.prd.cds.internal.unity3d.com/artifactory/api/npm/upm-candidates" | ||
| }, | ||
|
There was a problem hiding this comment. @TomasKiniulis fyi |
||
| "com.unity.render-pipelines.core": { | ||
| "version": "file:../../../com.unity.render-pipelines.core", | ||
| "depth": 0, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,12 +42,37 @@ public static bool useTexArray | |
| case GraphicsDeviceType.Vulkan: | ||
| return true; | ||
|
|
||
| // VFX basically depends on texture arrays, so TextureXR should be enabled even on platforms without SPI support | ||
| // SPI technically available on Metal, but due to fragmented OS/GPU matrix, let's not enable it yet | ||
sebastienlagarde marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| case GraphicsDeviceType.Metal: | ||
| return true; | ||
|
|
||
| default: | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Must be in sync with shader define in TextureXR.hlsl | ||
| /// <summary> | ||
| /// Returns true if the XR system uses texture arrays for MSAA. | ||
| /// </summary> | ||
| public static bool useTexArrayMSAA | ||
| { | ||
| get | ||
| { | ||
| switch (SystemInfo.graphicsDeviceType) | ||
| { | ||
| // Metal supports texture arrays with MSAA, but due to fragmented OS/GPU matrix, let's not enable it yet | ||
| case GraphicsDeviceType.Metal: | ||
| return false; | ||
|
|
||
| default: | ||
| return useTexArray; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Dimension of XR textures. | ||
| /// </summary> | ||
|
|
@@ -60,6 +85,18 @@ public static TextureDimension dimension | |
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Dimension of XR MSAA textures. | ||
| /// </summary> | ||
| public static TextureDimension dimensionMSAA | ||
|
There was a problem hiding this comment. Please do no add this extra API just for Metal. There was a problem hiding this comment. Ok.. so we've had these known issues in Spaceship demo's main menu (an on purpose screen glitch is gray instead of coloured) and VFX sparkles not bouncing off from the surfaces as expected, and we've had people working on this project and talking about this limitation/difference against Metal backend a year now? :) All discussions so far that I've seen suggest that there's something in Metal that should be fixed. But we really can't move on to enabling SPI/XR stuff anytime soon, and on some platforms you shouldn't expect XR/SPI at all. It makes sense for VFX to share TextureXR, but it's showing this problem where it's clearly depending on an API that locks you into XR/SPI assumptions, VFXGraph should work on targets without SPI too, where you still have texture arrays available. There was a problem hiding this comment. Yes problem with this is that the "correct" fix is tricky:
Or we can also solve it by not having TextureXR leak outside of shader code which means SPR API to access camera buffer always return Texture2D (Was the case before but it was changed for RTX I dont exactly remember why). We could also fix the interop issues by allowing a Texture2D view on a texture2DArray first slice... But I agree with Fabien that fixing it just by changing the type in metal is not an actual fix. Issues still remain for other platforms that dont support XR like XBox for instance. There was a problem hiding this comment. And also a problem of when stuff can get properly fixed vs having something working in the meanwhile :) My changes are not super intrusive though within TextureXR, and it will allow fixing the same issue on other platforms without XR/SPI as needed, but of course I tried to restrict actual changes to Metal runtime only when possible There was a problem hiding this comment. The MSAA texture array support situation is more unfortunate though. We always have features that "not everyone" depends on automatically, but even on current desktop mac gpu feature table, the availability depends too much to make it a hard requirement. For iOS it's simply not available at all There was a problem hiding this comment. Sure, but it will add a new public API that we'll have to maintain for many years ... let's please avoid that and fix the root issue. There was a problem hiding this comment. @fabien-unity how do we fix the root issue of having a Graphics Device without MSAA texture support, other than not using it when it's not available? :) It's a less common problem than limited XR/SPI availability, but equally could happen elsewhere too than just Metal. The main interest for HDRP users is deferred rendering in any case There was a problem hiding this comment. @fabien-unity Xbox issue is tracked via a single ticket for some failing graphics tests. specifically the depth collision one. https://fogbugz.unity3d.com/f/cases/1256635/ We can have a meeting to talk about how to fix the TextureXR issue. @JulienIgnace-Unity you might want to be here. |
||
| { | ||
| get | ||
| { | ||
| // TEXTURE2D_X_MSAA/LOAD_TEXTURE2D_X_MSAA macros will now expand to TEXTURE2D or TEXTURE2D_ARRAY | ||
| return useTexArrayMSAA ? TextureDimension.Tex2DArray : TextureDimension.Tex2D; | ||
| } | ||
| } | ||
|
|
||
| // Need to keep both the Texture and the RTHandle in order to be able to track lifetime properly. | ||
| static Texture m_BlackUIntTexture2DArray; | ||
| static Texture m_BlackUIntTexture; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TomasKiniulis FYI