feat: ask the graphics device what it provides, instead of comparing profiles - #3368
feat: ask the graphics device what it provides, instead of comparing profiles#3368sasvdw wants to merge 13 commits into
Conversation
A capability query answers only "can I do this now", so each renderer learns of a shortfall when it runs. The codebase has 17 such checks and 5 different outcomes. A declaration carries the same answer earlier, so something else can report every shortfall together. The declarer evaluates its own condition and records the outcome. A list of predicates cannot express a need that depends on configuration, or a clamp. Severity separates the two cases. Preferred means the declarer has a fallback. Required means it cannot run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The compositor subscribes to RenderContext.RendererInitialized, which already fires for every renderer and render feature. Nothing has to walk the graph. The report goes out at the end of the first DrawCore. That is the earliest complete point: PreDrawCoreInternal also calls EnsureContext, so an image effect initializes when it first draws, not when it collects. The report also logs the adapter and the feature set. Neither was logged before. GraphicsDeviceFeatures.ToString had no caller at all. Unmet Required requirements throw together and name each renderer. Before this, the first renderer to reach its own check decided how the game failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two call sites, one of each severity. ForwardRenderer already resolved its multisample count against the device. It now reports that decision. Only one of its three downgrade paths warned, so a project asking for 4x could silently get none. VoxelRenderer threw from Collect and returned silently from Draw, on the same condition. Both now read one flag, so the two paths agree. Its declaration keeps the profile comparison, so no device changes behaviour. The reason string says the comparison stands in for a query we do not have. VoxelRenderer is a plain data object, not a RendererCoreBase, so its owner has to pass the collector down. That gap is left visible on purpose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Require and Prefer took a free-form string and a caller-computed bool. Nothing central owned either, so neither call site named a real capability. One passed a stringified sample count. The other passed a profile comparison, which is the thing this design exists to replace. GraphicsCapability now owns identity and reads the answer from GraphicsDeviceFeatures. A renderer states what it needs, not whether it has it. Two kinds exist, both backed by data the engine already queries: a device feature flag, and multisampling of one pixel format at one sample count. A tier fits the same base once a backend has tier data. Kind and Name stay separate. A backend implements a kind or it does not, while the device answers per instance. GraphicsDeviceFeatures gained IsImplementedByBackend. The Vulkan backend declares multisampling unimplemented, because Texture.Vulkan.cs and PipelineState.Vulkan.cs hardcode VkSampleCountFlags.Count1 and no MultisampleCount conversion exists. The outcome is three-valued as a result, so the report can name the backend instead of blaming the hardware. ForwardRenderer declares multisampling for both formats it renders. It no longer computes the answer, so the iOS override can no longer make a device capability report unmet for a reason that is not the device. VoxelRenderer declared a profile comparison that stood in for two questions. Compute is a runtime capability, so it declares that. Shader model 5 is a content target, already enforced when the effect compiler picks 5_0 or 4_0, so it does not belong in a device query. That changes behaviour. HasComputeShaders is queried only on Direct3D11 and is hardcoded true elsewhere, so voxelization now runs where the profile comparison refused. It is also coarser than voxelization needs, and the capability set has no way to say shader model 5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The collector did not earn its 320 lines. Of what it claimed: Earlier reporting was false. Declaration happened in InitializeCore, and a query there happens at the same moment. Earliness comes from asking at initialization instead of at the point of use, which a renderer does alone. Severity collapses. With a query, Required is a throw and Preferred is a degrade. Attribution was already there; ForwardRenderer names itself in its own warning. Only aggregation was real, and that is diagnostics rather than architecture. It goes, and a compositor that needs three unavailable things now reports them one at a time. What stays is the vocabulary, which is what makes the check honest. GraphicsBackend.Implements replaces the instance member on GraphicsDeviceFeatures. Backend support is static and needs no device, so build time can read it. A game's platform head project sets StridePlatform, and Stride.Platform.props derives the graphics API from it, so a build targets one backend and answers only for that one. Each backend supplies its own partial, and a missing one fails the build. GraphicsDeviceFeatures keeps the device half in Provides and combines both in Supports. The backend is asked first. What a device provides does not matter when the backend cannot drive it, and the two have different remedies: another device fixes one, and only a change to Stride fixes the other. ForwardRenderer keeps its clamp and reads Supports to say why it fell back. It now names the backend rather than reporting "not supported", which blamed the hardware for a gap in Stride. VoxelRenderer asks for compute shaders in Collect and stores the answer, so Draw agrees with it. The compositor loses the collector wiring and keeps one line: it logs the adapter and the device features once. Nothing logged either before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The report sat in GraphicsCompositor.InitializeCore, which is the wrong owner. GameBase.InitializeBeforeRun creates the device, then loads content, and the compositor is content. It is also a RendererCoreBase, so it does not initialize until its first draw. So a device fact was reported by a content object, one frame late, once per compositor rather than once per device. GraphicsDevice.Recreate rebuilds features on a reset and nothing re-initializes the compositor, so the report also went stale and never corrected itself. Per compositor is the wrong count. An editor runs roughly 2N + 1 compositors plus one per thumbnail key, across N + 2 devices, because the preview and thumbnail services each build their own device and request different profiles. The compositor can also be null: SceneSystem calls Draw on it with ?., and the editor, the preview game and the thumbnail generator each leave it null at times. GraphicsDevice.Recreate is the single point where features are built, on both the create and the reset path, so the report belongs there. The partial class had no shared logger, and the two Direct3D partials each declared their own. The declaration moves to GraphicsDevice.cs, where one serves all four backends. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects in the previous commits of this branch. VoxelRenderer replaced a throw with a bare return, so a user who added the voxel renderer and a volume got no lighting and no reason. It now warns once. Collect runs every frame, so the report is latched, the way SceneCameraRenderer latches its missing-camera warning. ForwardRenderer computed its reason from the color format alone, but the clamp can come from the depth format or from the depth shader-resource flag, so it could name a cause it had not checked. It now names the constraint that bit. The iOS clamp also ran after the warning, which made that path silent, so it moves above it. GraphicsDeviceFeatures.ToString printed RequestedProfile, HasComputeShaders, HasDoublePrecision, HasMultiThreadingConcurrentResources and HasDriverCommandLists. It printed none of CurrentProfile, HasDepthAsSRV, HasDepthAsReadOnlyRT, HasMultiSampleDepthAsSRV, HasResourceRenaming or HasSRgb, which is every flag that makes a renderer quietly do something else. The device report added in the previous commit could therefore not explain a single degrade it was meant to explain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neither is about capabilities. Both turned up while reading how renderers react when one is missing, which is the argument for doing this work. LightShafts read forwardLightingFeature.ShadowMapRenderer without a guard and dereferenced it while drawing. ForwardLightingRenderFeature treats that property as optional and null-checks it twice, and GraphicsCompositorHelper builds the feature without one below profile 10.0. The result was a NullReferenceException in the draw loop. It now throws where it initializes, beside the two throws that already check for a missing mesh render feature and a missing forward lighting feature. CheckMipLevels on Direct3D 11 tested HasFlag(TextureFlags.DepthStencil) where Direct3D 12 and Vulkan test the negation. The documented purpose is to work around DXT images smaller than 4x4 on Direct3D 9 hardware, and a depth-stencil texture is never block compressed, so the condition never fired and the workaround was dead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Audited all twenty profile comparisons. Seven are device questions and change here. The other thirteen are not. A new Index32Bits capability replaces the comparison in GeometricPrimitive and PrimitiveProceduralModelBase. Direct3D 11 defines the flag as the condition it replaces, so that backend is unchanged. It does fix Vulkan, where CurrentProfile is the request echoed back, so a project asking for 9.x could not build a 32-bit-index primitive. Three backend sites now read the flag they were re-deriving. The rest stay. EffectSystem is the content target itself. GraphicsDeviceManager compares request to request. The Stride.Assets sites have no device. MaterialSpecularMicrofacetEnvironmentGGXLUT.Generate runs at build time too, where there is none to ask. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The per-format table was filled with None. Only MultisampleCountMax is load-bearing there: ForwardRenderer and Texture.InitializeFrom read it raw, and None is what keeps this backend's unimplemented multisampling unreachable. FormatSupport has no reader at all, so filling it from vkGetPhysicalDeviceFormatProperties changes no behaviour and unblocks the questions that were waiting on it. MultisampleCountMax stays None on purpose. Only flags the query answers precisely are mapped; Texture1D, Texture3D and TextureCube need the image format query instead. TryConvertPixelFormat is new because ConvertPixelFormat throws on formats it does not cover. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DeviceFeatureCapability.IsProvidedByDevice ends in a default arm that throws, so the compiler cannot report a kind that nothing answers for. Three facts, no device needed: every kind has a capability, no two capabilities share a kind, and every flag capability reaches a switch arm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
HasIndex32Bits decides between 16-bit and 32-bit index buffers with no other signal, and ToString did not report it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| HasDepthAsSRV = CurrentProfile >= Level_10_0; | ||
| HasDepthAsReadOnlyRT = CurrentProfile >= Level_11_0; | ||
| HasMultiSampleDepthAsSRV = CurrentProfile >= Level_11_0; | ||
| HasIndex32Bits = CurrentProfile > Level_9_3; |
There was a problem hiding this comment.
Not important, as DX9 is already removed as well as OpenGL, but...
shouldn't this be a >=? I remember using 32-bit indices on Direct3D 9.0c
| public static bool TryConvertPixelFormat(PixelFormat inputFormat, out VkFormat format) | ||
| { | ||
| try | ||
| { | ||
| ConvertPixelFormat(inputFormat, out format, out _, out _); | ||
| return true; | ||
| } | ||
| catch (InvalidOperationException) | ||
| { | ||
| format = VkFormat.Undefined; | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
I would do this the other way around, i.e., having the TryConvertPixelFormat as the "cheap" method that determines the conversion and then doing ConvertPixelFormat as the stub that calls TryConvertPixelFormat and throwns an exception when unsuccesful.
That way the Try... variant is the cheap one. And I also would document the exception.
There was a problem hiding this comment.
I agree with this. Been looking at it all for too long to realise.
There was a problem hiding this comment.
Done in f442c37.
TryConvertPixelFormat now holds the switch and returns false for a format with no Vulkan equivalent; ConvertPixelFormat calls it and throws, with an <exception> tag on it as you suggested.
Worth noting the cost was not only stylistic: GraphicsDeviceFeatures.Vulkan walks all 256 PixelFormat values at device creation, and most have no Vulkan equivalent, so the old wrapper threw and caught an InvalidOperationException for nearly every one of them.
All four existing signatures are unchanged, so every call site compiles as before — the only addition is a four-argument TryConvertPixelFormat overload that carries the conversion, with the two-argument one delegating to it. Verified by building the Vulkan variant specifically (-p:StrideGraphicsApis=Vulkan), since the file is behind STRIDE_GRAPHICS_API_VULKAN and a default Direct3D11 build does not compile it.
Still looking at your other two comments.
| /// A backend implements a kind or it does not. The device answers per capability instance, because | ||
| /// multisampling support depends on the pixel format and the sample count. | ||
| /// </remarks> |
There was a problem hiding this comment.
The remark about "multisampling support" is too specific for an enum GraphicsCapabilityKind that describes capabilities in the general sense.
I mean, explain that "backends" declare they support specific capabilities, but the user device can or cannot support some of them.
|
As I already said, I'm not opposed to moving from a feature level to a capability-based system if done well. I think however this may be a big breaking change. Right now we are at Anyway, I have a few worries:
I like where this goes, but I feel it a bit over-abstracted for not the correct reasons. A direct way to query the backend, and a direct way for the device to answer the "supports" questions (that would also ask the backend) could be lighter while offering the same functionality. That's my honest opinion. @xen2 What's your take on this? |
|
Yes, this new capability system will be for 4.5. I would like to take some time to make sure it's designed properly before moving forward. Also I agree with several points from @Ethereal77 I will focus on 4.4 release first, then I will come back on this topic, so please wait a few days/weeks. |
TryConvertPixelFormat called ConvertPixelFormat and caught the exception, so the cheap method paid for the expensive one. GraphicsDeviceFeatures walks all 256 pixel formats at device creation, and most of them have no Vulkan format, so it threw and caught an exception for nearly every one. TryConvertPixelFormat now holds the conversion and gives false for a format that does not convert. ConvertPixelFormat calls it and throws, and it now documents the exception.
3 participants
PR Details
Summary — Adds a capability vocabulary to
Stride.Graphics, so a renderer asks what the deviceprovides instead of comparing graphics profiles.
What it adds
Three questions that Stride currently answers with one number:
GraphicsCapabilitySupportisAvailable,NotProvidedByDeviceorNotImplementedByBackend. Thatthird state is the point. The two failures have different remedies — another device fixes one, and
only a change to Stride fixes the other — and there was no way to say so. The Vulkan backend has never
implemented multisampling, and today reports it as hardware that cannot do it.
A
GraphicsCapabilityowns its identity and how to read it fromGraphicsDeviceFeatures, so arenderer states what it needs rather than whether it has it. Two kinds so far: a device feature flag,
and
Multisampling(format, count), which is per pixel format and graded.Why
A feature-level ladder assumes capabilities are ordered and cumulative. Raytracing, mesh shaders and
variable-rate shading are optional and orthogonal — a card can be feature level 12_1 with no
raytracing. A profile comparison cannot ask about those, and it cannot tell hardware that will not do
something from a backend that does not do it yet.
One behaviour change
VoxelRenderernow asks for compute shaders instead of comparing profiles.HasComputeShadersisqueried only on Direct3D 11 and hardcoded
trueon the other backends, so voxelization now runs wherethe profile comparison refused it. The capability is also coarser than voxelization needs, because the
set cannot yet say shader model 5. This one needs a review.
Call sites
The engine compares a profile against a level in twenty-five places. Seven now ask the device. The
eighteen that stay are three different things, and only one of them is "not a device question".
Nine have no device to ask. The asset compilers and
MaterialSpecularMicrofacetEnvironmentGGXLUT.Generaterun at build time, and two backend sites pick aprofile before
GraphicsDeviceFeaturesexists. No query can replace these.Seven are device questions with no capability yet.
SamplerState.Direct3D11.cs:87caps anisotropyon 9.1,
Texture.Direct3D11.cs:1015asks for the standard multisample quality pattern, and the threeCheckMipLevelsworkarounds ask about a Direct3D 9 driver limit. These want the query. They keep thecomparison because the capability set has no term for what they ask.
Two read a profile that is not the right source,
GraphicsCompositorHelper.cs:60andLightShaderGroupDynamic.cs:94. The second is below.Two further sites read a profile without comparing it:
EffectSystem.cs:145is the content target,and
GraphicsDeviceManager.cs:624compares one request to another. Both are right as they are.Fixed
VK_INDEX_TYPE_UINT32is core Vulkan.
CurrentProfileis a device fact only on Direct3D 11; Vulkan echoes the request.LightShaftsdereferenced a nullShadowMapRendererat draw time.ForwardLightingRenderFeaturetreats it as optional, andGraphicsCompositorHelperbuilds thefeature without one below profile 10.0. It now throws where it initialises.
CheckMipLevelswas inverted on Direct3D 11. The workaround targets block-compressed textures,which are never depth-stencil, so the condition never fired. Direct3D 12 and Vulkan have the
!.FormatSupportnow comes fromvkGetPhysicalDeviceFormatProperties.MultisampleCountMaxdeliberately staysNone, becauseForwardRendererandTexture.InitializeFromread it raw and it is what keeps the unimplementedmultisampling path unreachable.
GraphicsDeviceFeatures.ToString()had no caller anywhere.GraphicsDevice.Recreatenow logs it, on both the create and the reset path, andToString()gainedthe flags it omitted — every one that makes a renderer quietly do something else.
Found, not fixed
Texture.Vulkan.cs:670truncates compressed mip chains on Vulkan when a project targets 9.x.LightShaderGroupDynamic.cs:39ignoresShaderProfile, so its permutation count can disagree withwhat
EffectSystemcompiles.What it does not do
Nothing touches
GraphicsProfile, the asset pipeline or serialized settings. The profile stays thebuild-time content target and the device-creation target, which no capability query can replace. Only
one new capability is added. The seven sites that want the query each need a different one, and each
of those deserves a considered design rather than a term bolted on here.
Related Issue
Follows the discussion on #3302 about the feature level system and moving toward capabilities.
Types of changes
Checklist
Validation status
StrideGraphicsApi=Direct3D11,=Direct3D12and=Vulkan. Not=Null—that backend does not compile on
mastertoday, with partial-method signature drift inNull/CommandList.Null.csandNull/Texture.Null.cs. TheGraphicsBackend.Null.cspartial this PRadds is therefore unverified, and an earlier claim here that Null built clean was an incremental
no-op on my part.
TestGraphicsCapability, all passing, no device needed. They guard how theset grows: a
GraphicsCapabilityKindthat nothing answers for, two capabilities on one kind, and akind with no switch arm, which throws today because the switch has a default arm the compiler
accepts. They do not validate this change.
Vulkan on Linux — at 17 of 17 each, plus both packaging jobs. No frame drifted past its LPIPS
baseline and none deferred to the vision gate.
and renders.
The device report makes it clear what that run did and did not cover. Every sample logs:
The samples render at profile 9_3, so they do exercise the paths this PR changes most:
CheckMipLevelsnow fires where the missing
!kept it unreachable, the depth-SRV flags readFalse, andHasIndex32BitsisFalseexactly as the oldCurrentProfile <= Level_9_3was. Gold images stillmatch, so none of those changed what is drawn.
Two things the suite does not reach: the
VoxelRendererbehaviour change, because no fixture usesvoxel GI, and multisampling, because every fixture renders with
MSAALevel: None.A game run from the editor on real hardware covers the other side of every one of those flags: