From f532624dc1604ab9c4b575e7f1a7fdad3e3893ba Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Sun, 12 Apr 2026 21:10:27 +1000 Subject: [PATCH] Two-sided lighting, rename misleading draw-count stat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs conflated as "weird colors": 1. Two-sided lighting. IFC placements often embed reflection matrices (mirrored families). Transforming a_normal by mat3(inst.transform) produces a normal pointing the wrong way on those instances, and max(n·L, 0) then clamps the surface to pure ambient — reads as dark / washed out. Use gl_FrontFacing to flip n in the fragment shader so both winding orientations shade correctly. The proper fix (ship an inverse-transpose normal matrix or a det-sign bit per instance) is still owed; that would unlock re-enabling GL_CULL_FACE for a big fragment- work win on closed solids. 2. Stats label "inst_draws" was counting indirect sub-draws, not actual GL draw calls — misleading since MDI collapses N sub- draws into one glMultiDrawElementsIndirect. Split into gl_draw_calls (real GL calls, = drawn-model count) and indirect_sub_draws (packed sub-commands). For a BIM model with 47k unique meshes at full view this now correctly reads "1 gl_draws (47092 sub)" rather than suggesting 47k driver dispatches. Co-Authored-By: Claude Opus 4.6 --- src/ifcviewer/MainWindow.cpp | 6 ++++-- src/ifcviewer/ViewportWindow.cpp | 19 ++++++++++++++----- src/ifcviewer/ViewportWindow.h | 6 ++++-- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/ifcviewer/MainWindow.cpp b/src/ifcviewer/MainWindow.cpp index ceeedc8cbd..8b63f3bdf6 100644 --- a/src/ifcviewer/MainWindow.cpp +++ b/src/ifcviewer/MainWindow.cpp @@ -41,13 +41,15 @@ MainWindow::MainWindow(QWidget* parent) connect(viewport_, &ViewportWindow::frameStatsUpdated, this, [this](const ViewportWindow::FrameStats& s) { if (!stats_label_->isVisible()) return; stats_label_->setText( - QString("%1 fps | %2 ms | %3/%4 obj | %5/%6 tri") + QString("%1 fps | %2 ms | %3/%4 obj | %5/%6 tri | %7 gl_draws (%8 sub)") .arg(s.fps, 0, 'f', 1) .arg(s.frame_time_ms, 0, 'f', 1) .arg(s.visible_objects) .arg(s.total_objects) .arg(s.visible_triangles) - .arg(s.total_triangles)); + .arg(s.total_triangles) + .arg(s.gl_draw_calls) + .arg(s.indirect_sub_draws)); }); connect(&AppSettings::instance(), &AppSettings::showStatsChanged, this, [this](bool show) { diff --git a/src/ifcviewer/ViewportWindow.cpp b/src/ifcviewer/ViewportWindow.cpp index b24ff7e3b3..d58f192733 100644 --- a/src/ifcviewer/ViewportWindow.cpp +++ b/src/ifcviewer/ViewportWindow.cpp @@ -123,7 +123,13 @@ uniform vec3 u_light_dir; out vec4 frag_color; void main() { + // Two-sided lighting: IFC placements frequently embed reflections + // (mirrored families), which flip triangle winding and invert the + // transformed normal. Taking abs(dot) — or equivalently flipping n + // based on gl_FrontFacing — makes both sides shade correctly + // regardless of winding / reflection state. vec3 n = normalize(v_normal); + if (!gl_FrontFacing) n = -n; float ndotl = max(dot(n, u_light_dir), 0.0); float ambient = 0.25; float diffuse = 0.75 * ndotl; @@ -906,7 +912,8 @@ void ViewportWindow::render() { visible_triangles_ = 0; visible_objects_ = 0; - instanced_draws_ = 0; + gl_draw_calls_ = 0; + indirect_sub_draws_ = 0; for (auto& [model_id, m] : models_gpu_) { if (m.hidden || !m.ssbo || m.ssbo_instance_count == 0) continue; @@ -926,7 +933,8 @@ void ViewportWindow::render() { visible_triangles_ += (cmd.count / 3) * cmd.instanceCount; visible_objects_ += cmd.instanceCount; } - instanced_draws_ += m.indirect_command_count; + indirect_sub_draws_ += m.indirect_command_count; + ++gl_draw_calls_; } gl_->glBindBuffer(GL_DRAW_INDIRECT_BUFFER, 0); @@ -964,16 +972,17 @@ void ViewportWindow::render() { stats.total_triangles = total_tri; stats.visible_triangles = visible_triangles_; stats.unique_meshes = total_meshes; - stats.instanced_draws = instanced_draws_; + stats.gl_draw_calls = gl_draw_calls_; + stats.indirect_sub_draws = indirect_sub_draws_; emit frameStatsUpdated(stats); qDebug("[frame] %.1f fps %.2f ms obj %u/%u tri %u/%u " - "meshes %u inst_draws %u " + "meshes %u gl_draws %u sub_draws %u " "vram %.1f MB (vbo %.1f + ebo %.1f + ssbo %.1f) models %zu (%zu hidden)", last_fps_, 1000.0f / last_fps_, visible_objects_, total_obj, visible_triangles_, total_tri, - total_meshes, instanced_draws_, + total_meshes, gl_draw_calls_, indirect_sub_draws_, (total_vbo + total_ebo + total_ssbo) / (1024.0*1024.0), total_vbo / (1024.0*1024.0), total_ebo / (1024.0*1024.0), diff --git a/src/ifcviewer/ViewportWindow.h b/src/ifcviewer/ViewportWindow.h index 966761eeaf..2c3019eb15 100644 --- a/src/ifcviewer/ViewportWindow.h +++ b/src/ifcviewer/ViewportWindow.h @@ -136,7 +136,8 @@ public: uint32_t total_triangles; uint32_t visible_triangles; uint32_t unique_meshes; - uint32_t instanced_draws; + uint32_t gl_draw_calls; // actual glMultiDrawElementsIndirect issues per frame + uint32_t indirect_sub_draws; // total commands packed into those indirect buffers }; signals: @@ -202,7 +203,8 @@ private: // Per-frame stats uint32_t visible_triangles_ = 0; uint32_t visible_objects_ = 0; - uint32_t instanced_draws_ = 0; + uint32_t gl_draw_calls_ = 0; + uint32_t indirect_sub_draws_ = 0; // Reused scratch: visible-instance index lists per mesh, flattened into // `visible_flat_` for upload. Both live in the parent object to avoid