Skip to content

Commit ff2e970

Browse files
Removed all D3D12 exclusions from tests. (#1665)
Re-enables the scissor test cases that were previously excluded on D3D12 due to an automatic mipmap generation issue in bgfx. --------- Co-authored-by: Branimir Karadzic <branimirkaradzic@gmail.com>
1 parent 167daf3 commit ff2e970

6 files changed

Lines changed: 42 additions & 29 deletions

File tree

Apps/Playground/Scripts/config.json

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -328,32 +328,20 @@
328328
{
329329
"title": "Scissor test",
330330
"playgroundId": "#W7E7CF#34",
331-
"referenceImage": "scissor-test.png",
332-
"excludedGraphicsApis": [ "D3D12" ],
333-
"comments": {
334-
"D3D12": "reenable when automatic mip-maps issue is fixed in bgfx"
335-
}
331+
"referenceImage": "scissor-test.png"
336332
},
337333
{
338334
"title": "Scissor test with 0.9 hardware scaling",
339335
"playgroundId": "#W7E7CF#34",
340336
"replace": "//options//, hardwareScalingLevel = 0.9;",
341337
"referenceImage": "scissor-test-2.png",
342-
"excludedGraphicsApis": [ "D3D12" ],
343-
"errorRatio": 50,
344-
"comments": {
345-
"D3D12": "reenable when automatic mip-maps issue is fixed in bgfx"
346-
}
338+
"errorRatio": 50
347339
},
348340
{
349341
"title": "Scissor test with 1.5 hardware scaling",
350342
"playgroundId": "#W7E7CF#34",
351343
"replace": "//options//, hardwareScalingLevel = 1.5;",
352-
"referenceImage": "scissor-test-3.png",
353-
"excludedGraphicsApis": [ "D3D12" ],
354-
"comments": {
355-
"D3D12": "reenable when automatic mip-maps issue is fixed in bgfx"
356-
}
344+
"referenceImage": "scissor-test-3.png"
357345
},
358346
{
359347
"title": "Scissor test with negative x and y",

CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ FetchContent_Declare(base-n
3535
EXCLUDE_FROM_ALL)
3636
FetchContent_Declare(bgfx.cmake
3737
GIT_REPOSITORY https://github.com/BabylonJS/bgfx.cmake.git
38-
GIT_TAG be466af2a964bf2d2d28fdaf1543d89129c7fe21
38+
GIT_TAG a0adee5ccad355a0b91f9247402ed422aaee4f2e
3939
EXCLUDE_FROM_ALL)
4040
FetchContent_Declare(CMakeExtensions
4141
GIT_REPOSITORY https://github.com/BabylonJS/CMakeExtensions.git

Plugins/NativeEngine/Source/NativeEngine.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1742,7 +1742,13 @@ namespace Babylon
17421742

17431743
if (texture != nullptr)
17441744
{
1745-
attachments[numAttachments++].init(texture->Handle());
1745+
const bgfx::Caps* caps = bgfx::getCaps();
1746+
// bgfx validation now asserts when trying to use BGFX_RESOLVE_AUTO_GEN_MIPS with a texture that doesn't have the BGFX_CAPS_FORMAT_TEXTURE_MIP_AUTOGEN flag,
1747+
// but before it would just ignore the flag and not generate mips without any warning. This prevents validation assert, but rendering might be broken if autogen
1748+
// mips were expected. Basically this change preserves previous behavior.
1749+
attachments[numAttachments++].init(texture->Handle(), bgfx::Access::Write, 0, 1, 0
1750+
, 0 != (caps->formats[texture->Format()] & BGFX_CAPS_FORMAT_TEXTURE_MIP_AUTOGEN) ? BGFX_RESOLVE_AUTO_GEN_MIPS : BGFX_RESOLVE_NONE
1751+
);
17461752
}
17471753

17481754
bgfx::TextureHandle depthStencilTextureHandle = BGFX_INVALID_HANDLE;
@@ -1772,7 +1778,7 @@ namespace Babylon
17721778
// only allows mipmaps resolve step when mipmapping is asked and for the color texture, not the depth.
17731779
// https://github.com/bkaradzic/bgfx/blob/2c21f68998595fa388e25cb6527e82254d0e9bff/src/renderer_d3d11.cpp#L4525
17741780
depthStencilAttachmentIndex = numAttachments;
1775-
attachments[numAttachments++].init(depthStencilTextureHandle);
1781+
attachments[numAttachments++].init(depthStencilTextureHandle, bgfx::Access::Write, 0, 1, 0, BGFX_RESOLVE_NONE);
17761782
}
17771783

17781784
bgfx::FrameBufferHandle frameBufferHandle = bgfx::createFrameBuffer(numAttachments, attachments.data());

Plugins/NativeXr/Source/NativeXrImpl.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,15 +275,22 @@ namespace Babylon
275275
arcana::make_task(m_sessionState->GraphicsContext.AfterRenderScheduler(), arcana::cancellation::none(), [colorTexture, depthTexture, &viewConfig]() {
276276
bgfx::overrideInternal(colorTexture, reinterpret_cast<uintptr_t>(viewConfig.ColorTexturePointer));
277277
bgfx::overrideInternal(depthTexture, reinterpret_cast<uintptr_t>(viewConfig.DepthTexturePointer));
278-
}).then(m_runtimeScheduler, m_sessionState->CancellationSource, [this, thisRef{shared_from_this()}, colorTexture, depthTexture, requiresAppClear, &viewConfig]() {
278+
}).then(m_runtimeScheduler, m_sessionState->CancellationSource, [this, thisRef{shared_from_this()}, colorTexture, depthTexture, colorTextureFormat, requiresAppClear, &viewConfig]() {
279279
const auto eyeCount = std::max(static_cast<uint16_t>(1), static_cast<uint16_t>(viewConfig.ViewTextureSize.Depth));
280280
// TODO (rgerd): Remove old framebuffers from resource table?
281281
viewConfig.FrameBuffers.resize(eyeCount);
282282
for (uint16_t eyeIdx = 0; eyeIdx < eyeCount; eyeIdx++)
283283
{
284+
// See NativeEngine::CreateFrameBuffer: gate BGFX_RESOLVE_AUTO_GEN_MIPS on format caps and
285+
// always pass BGFX_RESOLVE_NONE for depth (depth formats don't support autogen mips).
286+
const bgfx::Caps* caps = bgfx::getCaps();
287+
const uint8_t colorResolve = 0 != (caps->formats[colorTextureFormat] & BGFX_CAPS_FORMAT_TEXTURE_MIP_AUTOGEN)
288+
? BGFX_RESOLVE_AUTO_GEN_MIPS
289+
: BGFX_RESOLVE_NONE;
290+
284291
std::array<bgfx::Attachment, 2> attachments{};
285-
attachments[0].init(colorTexture, bgfx::Access::Write, eyeIdx);
286-
attachments[1].init(depthTexture, bgfx::Access::Write, eyeIdx);
292+
attachments[0].init(colorTexture, bgfx::Access::Write, eyeIdx, 1, 0, colorResolve);
293+
attachments[1].init(depthTexture, bgfx::Access::Write, eyeIdx, 1, 0, BGFX_RESOLVE_NONE);
287294

288295
auto frameBufferHandle = bgfx::createFrameBuffer(static_cast<uint8_t>(attachments.size()), attachments.data(), false);
289296

Polyfills/Canvas/Source/Canvas.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,17 @@ namespace Babylon::Polyfills::Internal
168168
bgfx::createTexture2D(m_width, m_height, false, 1, bgfx::TextureFormat::RGBA8, BGFX_TEXTURE_RT, mem),
169169
bgfx::createTexture2D(m_width, m_height, false, 1, bgfx::TextureFormat::D24S8, BGFX_TEXTURE_RT)};
170170

171+
// See NativeEngine::CreateFrameBuffer: bgfx validation now asserts when BGFX_RESOLVE_AUTO_GEN_MIPS is used
172+
// with a texture whose format doesn't have BGFX_CAPS_FORMAT_TEXTURE_MIP_AUTOGEN. Gate the color attachment
173+
// on the capability and pass BGFX_RESOLVE_NONE for the depth attachment (depth formats never support autogen).
174+
const bgfx::Caps* caps = bgfx::getCaps();
175+
const uint8_t colorResolve = 0 != (caps->formats[bgfx::TextureFormat::RGBA8] & BGFX_CAPS_FORMAT_TEXTURE_MIP_AUTOGEN)
176+
? BGFX_RESOLVE_AUTO_GEN_MIPS
177+
: BGFX_RESOLVE_NONE;
178+
171179
std::array<bgfx::Attachment, textures.size()> attachments{};
172-
for (size_t idx = 0; idx < attachments.size(); ++idx)
173-
{
174-
attachments[idx].init(textures[idx]);
175-
}
180+
attachments[0].init(textures[0], bgfx::Access::Write, 0, 1, 0, colorResolve);
181+
attachments[1].init(textures[1], bgfx::Access::Write, 0, 1, 0, BGFX_RESOLVE_NONE);
176182
auto handle = bgfx::createFrameBuffer(static_cast<uint8_t>(attachments.size()), attachments.data(), true);
177183
assert(handle.idx != bgfx::kInvalidHandle);
178184
m_frameBuffer = std::make_unique<Graphics::FrameBuffer>(m_graphicsContext, handle, m_width, m_height, false, false, false);

Polyfills/Canvas/Source/FrameBufferPool.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,17 @@ namespace Babylon::Polyfills
5959
bgfx::createTexture2D(m_width, m_height, false, 1, bgfx::TextureFormat::RGBA8, BGFX_TEXTURE_RT | BGFX_SAMPLER_U_BORDER | BGFX_SAMPLER_V_BORDER | BGFX_SAMPLER_BORDER_COLOR(0), mem),
6060
bgfx::createTexture2D(m_width, m_height, false, 1, bgfx::TextureFormat::D24S8, BGFX_TEXTURE_RT | BGFX_SAMPLER_U_BORDER | BGFX_SAMPLER_V_BORDER | BGFX_SAMPLER_BORDER_COLOR(0))};
6161

62+
// See NativeEngine::CreateFrameBuffer: bgfx validation now asserts when BGFX_RESOLVE_AUTO_GEN_MIPS is used
63+
// with a texture whose format doesn't have BGFX_CAPS_FORMAT_TEXTURE_MIP_AUTOGEN. Gate the color attachment
64+
// on the capability and pass BGFX_RESOLVE_NONE for the depth attachment (depth formats never support autogen).
65+
const bgfx::Caps* caps = bgfx::getCaps();
66+
const uint8_t colorResolve = 0 != (caps->formats[bgfx::TextureFormat::RGBA8] & BGFX_CAPS_FORMAT_TEXTURE_MIP_AUTOGEN)
67+
? BGFX_RESOLVE_AUTO_GEN_MIPS
68+
: BGFX_RESOLVE_NONE;
69+
6270
std::array<bgfx::Attachment, textures.size()> attachments{};
63-
for (size_t idx = 0; idx < attachments.size(); ++idx)
64-
{
65-
attachments[idx].init(textures[idx]);
66-
}
71+
attachments[0].init(textures[0], bgfx::Access::Write, 0, 1, 0, colorResolve);
72+
attachments[1].init(textures[1], bgfx::Access::Write, 0, 1, 0, BGFX_RESOLVE_NONE);
6773
TextBuffer = bgfx::createFrameBuffer(static_cast<uint8_t>(attachments.size()), attachments.data(), true);
6874

6975
FrameBuffer = new Graphics::FrameBuffer(*m_graphicsContext, TextBuffer, m_width, m_height, false, false, false);

0 commit comments

Comments
 (0)