Editor model rendering optimizations and fixes #8694

matgis · 2024-03-21T15:51:32Z

Fixed a performance regression in the editor introduced by the recent addition of custom vertex attributes to models.
Fixed broken preview of model scene files (.gltf, .dae, etc.). Previously you'd only see the bounding boxes if you opened a .gltf file in the editor.
Fixed a memory leak of scene-cache data associated with the OpenGL context used for scene picking hit tests after closing an editor tab.
The editor will now share vertex buffers among meshes, as long as they do not use any world-space attributes.
The Coordinate Space setting of declared vertex attributes takes precedence, but the Vertex Space setting on the material will be used as the default for any undeclared vertex attributes where it has relevance.
The editor will now produce world-space normal attributes correctly, if desired.
The editor will no longer produce a broken normal matrix for objects that have a non-uniformly scaled parent.

matgis · 2024-03-25T07:59:32Z

editor/src/clj/editor/code/shader.clj

@@ -238,7 +238,6 @@

  (input included-proj-paths+full-lines ProjPath+Lines :array)

-  (output build-targets g/Any :cached produce-build-targets)


Unused output. Fun fact: produce-build-targets does not even exist in this file. We never check this in the defnode macro, and this situation will create an output that always produces nil. I started to add more validation to the defnode macro, but I stashed it since it was turning out to be a bit of a rabbit hole.

Reported as #8702.

matgis · 2024-03-25T08:46:11Z

editor/src/clj/editor/geom.clj

+            (.normalize n) ; Need to normalize since the matrix may be scaled.
+            [(.x n) (.y n) (.z n) 0.0])
+          normals)))
+


Moved here from model.clj.

matgis · 2024-03-25T08:48:43Z

editor/src/clj/editor/graphics.clj

+              (subvec default-element-values old-element-count new-element-count)))
+
+      :else
+      double-values)))


Bug: We used to put a zero in the W coordinate of positions as we copied X Y Z components from the mesh scene. This results in the points not being transformed correctly by the 4x4 transform matrix. For vectors (like normals or tangents), we should use 0.0, but for points it needs to be 1.0.

matgis · 2024-03-25T08:54:09Z

editor/src/clj/editor/graphics.clj

@@ -266,16 +310,18 @@
          :element-count 1}
         {:name "normal"
          :semantic-type :semantic-type-normal
+          :coordinate-space :coordinate-space-world


The presence of the :coordinate-space key on an attribute ensures we will assoc in the default-coordinate-space supplied to the shader-bound-attributes function below.

matgis · 2024-03-25T08:54:58Z

editor/src/clj/editor/graphics.clj

  (let [shader-bound-attribute? (comp boolean (shader/attribute-infos shader gl) :name)
        declared-material-attribute-key? (into #{} (map :name-key) material-attribute-infos)
        manufactured-attribute-infos (into []
                                           (comp (remove declared-material-attribute-key?)
                                                 (map attribute-key->default-attribute-info))
-                                           manufactured-stream-keys)
+                                           manufactured-attribute-keys)


There were some inconsistent naming in this file. I decided to use consistent terms everywhere.

matgis · 2024-03-25T09:56:56Z

editor/src/clj/editor/model_scene.clj

      (gl/gl-disable gl GL/GL_CULL_FACE)
-      (.glBlendFunc gl GL/GL_SRC_ALPHA GL/GL_ONE_MINUS_SRC_ALPHA)


No need. Since we're drawing an opaque mesh, we've disabled blending completely above.

matgis · 2024-03-25T10:05:09Z

editor/src/clj/editor/model_scene.clj

+            material-name (mesh-material-index->material-name material-index)]
+        {:aabb mesh-aabb
+         :material-name material-name
+         :renderable-data mesh-renderable-data}))))


We do less work in the render-fn now. We can prepare the renderable-datas and cache them in the renderable-mesh-set output before producing the scene.

Potentially we might be able to fully create and cache the populated VertexBuffers here, but currently we rely on getting the shader-bound-attributes from the OpenGL context that is only available to us in the render-fn. I think we may want to introduce a shared "resource" OpenGL context that we use to store vertex buffers and textures so they can be shared between scene views, but that is a larger change that could have potential issues with some hardware.

matgis · 2024-03-25T10:07:29Z

editor/src/clj/editor/model_scene.clj

-                                                   (recur (+ i 3) (geom/aabb-incorporate aabb p)))
-                                                 aabb))]
-                                    (recur aabb (next meshes)))
-                                  aabb))))


We now use the aabb-min and aabb-max values from the importer! 🌈

matgis · 2024-03-25T10:09:02Z

editor/src/clj/editor/model_scene.clj

-    (mesh->vb! vb world-transform vertex-space vertex-attribute-bytes mesh)))
+(defn- update-vb [^GL2 _gl ^VertexBuffer vb data]
+  (let [{:keys [mesh-renderable-data ^Matrix4d world-transform ^Matrix4d normal-transform vertex-attribute-bytes]} data]
+    (mesh->vb! vb world-transform normal-transform vertex-attribute-bytes mesh-renderable-data)


Added normal-transform, changed final argument to renderable-data for the mesh.

matgis · 2024-03-25T10:11:09Z

editor/src/clj/editor/model_scene.clj

-        vbuf (vtx/make-vertex-buffer vertex-description :dynamic (alength ^ints (:indices mesh)))]
+  (let [{:keys [mesh-renderable-data vertex-description]} data
+        position-data (:position-data mesh-renderable-data)
+        vbuf (vtx/make-vertex-buffer vertex-description :dynamic (count position-data))]


The :indices have been used to construct a non-indexed triangle list already in the renderable-mesh-set output, so at this point we can just count the number of positions.

matgis · 2024-03-25T10:16:38Z

editor/src/clj/editor/model_scene.clj

+  (let [mesh-renderable (:renderable mesh-scene)
+        material-name (:material-name mesh-renderable)
+        material-scene-info (material-name->material-scene-info material-name)
+        claimed-scene (scene/claim-child-scene old-node-id new-node-id new-node-outline-key mesh-scene)]


An issue with the old code was that the Model using a particular model-scene did not claim ownership of the scene-data produced by its scene output. This resulted in meshes that had different requirements in terms of materials and transforms sharing the same request-id for the scene-cache. There were also issues related to picking and selection, since the renderables could not be uniquely identified to belong to the referencing Model.

vlaaad

Did I read it? Yes. Did I get it? Well... :D

matgis · 2024-03-25T14:28:20Z

Haha, OK... I'll ask Jhonny to have a look as well. 😅

Jhonnyg · 2024-03-26T19:00:32Z

editor/src/clj/editor/graphics.clj

+(defn coordinate-space-info
+  "Returns a map of coordinate-space to sets of semantic-type that expect that
+  coordinate-space. Only :semantic-type-position and :semantic-type-normal
+  attributes are considered. We use this to determine if we need to include
+  a :world-transform or :normal-transform in the renderable-datas we supply to
+  the graphics/put-attributes! function. We can also use this information to
+  figure out if we should cancel out the world transform from the render
+  transforms we feed into shaders as uniforms in case the same shader is used to


excellent info!

Jhonnyg · 2024-03-26T19:03:30Z

editor/src/clj/editor/math.clj

+  TODO:
+  We should probably just deprecate this behavior, since it will confuse users
+  that mix local-space and world-space attributes of the same type."


Jhonnyg · 2024-03-26T19:04:32Z

editor/src/clj/editor/model.clj

-  (if (not scene)
+(g/defnk produce-scene [_node-id scene material-name->material-scene-info]
+  (if scene
+    (model-scene/augment-scene scene _node-id model-resource-type-label material-name->material-scene-info)


nice, didn't know this function existed!

Jhonnyg · 2024-03-26T19:10:35Z

editor/src/clj/editor/model_scene.clj

  (attribute vec2 texcoord0)
  (varying vec3 var_normal)
  (varying vec2 var_texcoord0)
  (defn void main []
-    (setq gl_Position (* gl_ModelViewProjectionMatrix position))
+    (setq gl_Position (* world_view_proj_matrix position))


Nice, the gl_ModelViewProjectionMatrix is an old GL remnant that we should move away from!

Jhonnyg · 2024-03-27T03:52:39Z

editor/src/clj/editor/scene.clj

@@ -870,6 +870,7 @@
      (.destroy drawable))
    (when-let [^GLAutoDrawable picking-drawable (g/node-value node-id :picking-drawable)]
      (gl/with-drawable-as-current picking-drawable
+        (scene-cache/drop-context! gl)


I didn't know we created a secondary context for the picking, why do we need that?

Editor model rendering optimizations and fixes

fb0e0c3

matgis added bug Something is not working as expected editor Issues related to the Defold editor model Issues related to 3D models performance labels Mar 21, 2024

matgis self-assigned this Mar 21, 2024

matgis commented Mar 25, 2024

View reviewed changes

matgis requested a review from vlaaad March 25, 2024 10:16

vlaaad approved these changes Mar 25, 2024

View reviewed changes

matgis requested a review from Jhonnyg March 25, 2024 14:28

Jhonnyg approved these changes Mar 27, 2024

View reviewed changes

matgis merged commit 99ebc50 into editor-dev Mar 27, 2024
6 checks passed

matgis deleted the DEFEDIT-8666-model-performance branch March 27, 2024 07:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editor model rendering optimizations and fixes #8694

Editor model rendering optimizations and fixes #8694

matgis commented Mar 21, 2024 •

edited

matgis Mar 25, 2024

matgis Mar 25, 2024

matgis Mar 25, 2024

matgis Mar 25, 2024

matgis Mar 25, 2024

matgis Mar 25, 2024

matgis Mar 25, 2024

matgis Mar 25, 2024

matgis Mar 25, 2024

matgis Mar 25, 2024

matgis Mar 25, 2024

vlaaad left a comment

matgis commented Mar 25, 2024

Jhonnyg Mar 26, 2024

Jhonnyg Mar 26, 2024

Jhonnyg Mar 26, 2024

Jhonnyg Mar 26, 2024

Jhonnyg Mar 27, 2024

		@@ -238,7 +238,6 @@

		(input included-proj-paths+full-lines ProjPath+Lines :array)

		(output build-targets g/Any :cached produce-build-targets)

		(gl/gl-disable gl GL/GL_CULL_FACE)
		(.glBlendFunc gl GL/GL_SRC_ALPHA GL/GL_ONE_MINUS_SRC_ALPHA)

Editor model rendering optimizations and fixes #8694

Editor model rendering optimizations and fixes #8694

Conversation

matgis commented Mar 21, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vlaaad left a comment

Choose a reason for hiding this comment

matgis commented Mar 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matgis commented Mar 21, 2024 •

edited