Make silent render failures loud, off the hot path

packages/flutter_scene/CHANGELOG.md

@@ -1,6 +1,12 @@
 ## 0.22.0
 
 * `Node.position`, `Node.rotation`, and `Node.scale` read and write the local transform one component at a time, and editing a returned copy in place throws in debug builds rather than silently doing nothing.
+* A frame that draws nothing now prints once in debug builds naming the likely cause (not ready, empty region, no views composite to screen, no visible meshes, or a layer mask matching nothing).
+* Degenerate cameras now assert in debug builds, catching a view direction of zero length, an `up` parallel to it, and a field of view passed in degrees.
+* The web backend now throws on a bind to a shader uniform or texture name that does not exist, matching native, instead of silently sampling whatever was bound last.
+* `SkinnedGeometry.uploadVertexData` and `setCustomAttribute` now throw on a buffer whose length does not match the vertex count, instead of rendering garbage.
+* A `Mesh` whose primitive geometry is replaced now recomputes its bounds on its own, rather than over-culling until `markLocalBoundsDirty` is called.
+* An `AnimationClip` that binds zero of its channels now asserts in debug builds, naming the wanted nodes, since it otherwise plays while nothing moves.
 * Fixed the second and every later `flutter run` rendering nothing, from two hook invocations writing the same shader bundle filenames with different backend trims.
 * Fixed `Scene.initializeStaticResources()` reporting ready when the shader bundle loaded but held nothing this engine can read.
 * Fixed `dart run flutter_scene:init` then `flutter run` failing on a new app with "Flutter failed to list directory".

packages/flutter_scene/lib/src/animation/animation_clip.dart

@@ -170,6 +170,32 @@ class AnimationClip {
       if (channelTarget == null) continue;
       _bindings.add(_ChannelBinding(channel, channelTarget));
     }
+    assert(_checkAnyChannelBound(target));
+  }
+
+  // Debug-only. Fires when the bind resolves zero channels against a
+  // non-empty animation, which almost always means the clip was bound to the
+  // wrong node (the mesh instead of the scene root) or the node names do not
+  // match the animation's targets. A partial bind (some channels hit, some
+  // miss) is a supported retarget onto a subset of the rig, so it stays
+  // silent rather than risk a false positive on a legitimate use.
+  bool _checkAnyChannelBound(Node target) {
+    if (_bindings.isNotEmpty || _animation.channels.isEmpty) {
+      return true;
+    }
+    final wanted = <String>{
+      for (final channel in _animation.channels) channel.bindTarget.nodeName,
+    };
+    final sample = wanted.take(5).join(', ');
+    final more = wanted.length > 5 ? ', and ${wanted.length - 5} more' : '';
+    throw StateError(
+      'AnimationClip bound 0 of ${_animation.channels.length} channels against '
+      '"${target.name}", so the clip will play and nothing will move. None of '
+      'the nodes this animation targets exist in that subtree; bind to the '
+      'subtree root that holds the animated nodes (usually the imported scene '
+      'root, not the mesh node), and check the names match. Nodes wanted: '
+      '$sample$more.',
+    );
   }
 
   /// Evaluates each bound channel at [playbackTime] and accumulates the

packages/flutter_scene/lib/src/camera.dart

@@ -135,7 +135,21 @@ abstract class Camera {
 }
 
 Matrix4 _matrix4LookAt(Vector3 position, Vector3 target, Vector3 up) {
-  Vector3 forward = (target - position).normalized();
+  final Vector3 viewDirection = target - position;
+  assert(
+    viewDirection.length2 > 1e-12,
+    'Camera target equals its position, so the view direction is undefined and '
+    'the scene renders empty. Move target away from position.',
+  );
+  assert(
+    up.cross(viewDirection).length2 > 1e-12,
+    'Camera up is parallel to the view direction (position toward target), so '
+    'the view basis is degenerate and the scene renders empty. Use an up vector '
+    'that is not parallel to the view direction; for a top-down or bottom-up '
+    'camera use Vector3(0, 0, 1) or Vector3(0, 0, -1) in place of '
+    'Vector3(0, 1, 0).',
+  );
+  Vector3 forward = viewDirection.normalized();
   Vector3 right = up.cross(forward).normalized();
   up = forward.cross(right).normalized();
 
@@ -165,6 +179,18 @@ Matrix4 _matrix4Perspective(
   double zNear,
   double zFar,
 ) {
+  assert(
+    fovRadiansY > 0 && fovRadiansY < pi,
+    'fovRadiansY is $fovRadiansY, which is not a valid vertical field of view '
+    'in RADIANS (it must be between 0 and pi). A value like 60 is degrees; pass '
+    '60 * degrees2Radians instead.',
+  );
+  assert(
+    zNear > 0 && zFar > zNear,
+    'The camera frustum is degenerate (near $zNear, far $zFar). near must be '
+    'greater than 0 and far must be greater than near, or the depth mapping '
+    'collapses and the scene renders empty or Z-fights.',
+  );
   double height = tan(fovRadiansY * 0.5);
   double width = height * aspectRatio;
 

packages/flutter_scene/lib/src/fmat/target_shader_bundle.dart

@@ -164,14 +164,22 @@ Future<void> buildTargetShaderBundleJson({
 /// Returns the backend set needed by [buildInput].
 ///
 /// A config with no code assets names no target OS, which is web and also the
-/// second invocation `flutter run` makes for a native target. Both resolve to
-/// GLES, so the native one must never overwrite the target build's outputs;
-/// [shaderBundleTargetKey] is what keeps them apart in the tree.
+/// data-asset-only invocation `flutter run` makes for a native target. Both
+/// resolve to GLES, so the native one must never overwrite the target build's
+/// outputs; [shaderBundleTargetKey] is what keeps them apart in the tree.
 ///
-/// TODO(hook-target-invocations): that second invocation still compiles and
-/// ships a GLES set no native app loads, roughly 1.5 MB in the bundle, because
-/// nothing in one hook input distinguishes it from a web build. Skipping it
-/// needs the invoker to say which it is.
+/// That native data-only pass still compiles and ships a GLES set no native app
+/// loads, roughly 1.5 MB. It cannot be dropped here. Its hook input is
+/// byte-identical to a real web build's (both just `data_assets/data`, no code
+/// config, no target OS), and web genuinely needs the GLES set, so nothing the
+/// hook can read tells the wasteful native pass from the required web one. The
+/// invoker knows the target platform but never puts it on a data-asset-only
+/// input. `generated_target_isolation_test.dart` locks that indistinguishability.
+///
+/// TODO(hook-target-invocations): dropping the waste needs an upstream change,
+/// the invoker naming the target platform (or final artifact) on data-asset-only
+/// inputs, or flutter_tools not making the redundant native data pass. Revisit
+/// if a Flutter release adds such a field.
 Set<ShaderBundleBackend> shaderBundleBackendsForBuild(BuildInput buildInput) =>
     shaderBundleBackendsForOS(
       buildInput.config.buildCodeAssets

packages/flutter_scene/lib/src/geometry/geometry.dart

@@ -301,6 +301,18 @@ abstract class Geometry {
         'must be between 1 and 4',
       );
     }
+    // _vertexCount is 0 until the first vertex upload, so a mismatch can only
+    // be judged once the count is known. Setting the attribute first is a
+    // legitimate ordering, covered by the message clause below.
+    if (_vertexCount > 0 && data.length != _vertexCount * components) {
+      throw ArgumentError(
+        'Custom attribute "$name" has ${data.length} floats, but this geometry '
+        'has $_vertexCount vertices at $components components each, so it needs '
+        '${_vertexCount * components}. Set the attribute after uploading '
+        'vertices, and re-set it after any rebuild that changes the vertex '
+        'count.',
+      );
+    }
     final bytes = ByteData.sublistView(data);
     final buffer = gpu.gpuContext.createDeviceBuffer(
       gpu.StorageMode.hostVisible,
@@ -350,6 +362,20 @@ abstract class Geometry {
     ByteData? indices, {
     gpu.IndexType indexType = gpu.IndexType.int16,
   }) {
+    final stride = _expectedVertexStrideInBytes;
+    if (stride != null && vertices.lengthInBytes != vertexCount * stride) {
+      throw ArgumentError(
+        'uploadVertexData got ${vertices.lengthInBytes} bytes for $vertexCount '
+        'vertices, but $runtimeType packs a $stride-byte vertex, so it needs '
+        '${vertexCount * stride} bytes. Repack at $stride bytes per vertex '
+        '(position 3, normal 3, tex_coords_0 2, tex_coords_1 2, color 4, '
+        'tangent 4'
+        '${stride == kSkinnedPerVertexSize ? ', joints 4, weights 4' : ''}, '
+        'all float32, in that order), or supply attributes as separate arrays '
+        'with MeshGeometry.fromArrays.',
+      );
+    }
+
     _cpuVertices = vertices;
     _cpuIndices = indices;
 
@@ -644,6 +670,11 @@ abstract class Geometry {
   /// importer didn't bake one (notably the runtime GLB importer).
   bool get _autoScanBoundsOnUpload => true;
 
+  /// The exact interleaved vertex stride [uploadVertexData] expects, or
+  /// null for a caller-defined layout it does not police. The unskinned
+  /// and skinned subclasses override it with their fixed strides.
+  int? get _expectedVertexStrideInBytes => null;
+
   /// Scan the position attribute (the first 12 bytes of each vertex,
   /// shared across the unskinned and skinned layouts)
   /// to populate [_localBounds] and [_localBoundingSphere].
@@ -929,6 +960,9 @@ class UnskinnedGeometry extends Geometry {
   // not flip this (they never change how the built-in attributes are stored).
   bool get _isDeInterleaved => _vertexStreams.length >= 2;
 
+  @override
+  int get _expectedVertexStrideInBytes => kUnskinnedPerVertexSize;
+
   @override
   List<ByteData> _vertexStreamBytes(ByteData vertices, int vertexCount) {
     final streams = InterleavedLayoutAdapter.splitUnskinnedAttributes(
@@ -1152,6 +1186,9 @@ class SkinnedGeometry extends Geometry {
   @override
   bool get _autoScanBoundsOnUpload => false;
 
+  @override
+  int get _expectedVertexStrideInBytes => kSkinnedPerVertexSize;
+
   @override
   void setJointsTexture(gpu.Texture? texture, int width) {
     _jointsTexture = texture;

packages/flutter_scene/lib/src/gpu/web/render_pass.dart

@@ -558,7 +558,19 @@ base class RenderPass {
       throw StateError('bindUniform called before bindPipeline');
     }
     final struct = slot.shader._uniformStructs[slot.uniformName];
-    if (struct == null) return;
+    if (struct == null) {
+      // Match the native backend, which throws when the shader has no uniform
+      // slot of this name. Swallowing it silently binds nothing and the draw
+      // reads stale uniform state, wrong only on web and only for some draw
+      // orders. The lookup reads the shader's live reflection, so it stays
+      // correct across hot reload.
+      throw StateError(
+        'Failed to bind uniform. This shader declares no uniform block named '
+        '"${slot.uniformName}". A block binds by its type name, not its '
+        'instance name, and a block nothing in the shader reads is optimized '
+        'out by the compiler and reflects as absent.',
+      );
+    }
 
     final gl = _gpuContext._gl;
 
@@ -680,7 +692,20 @@ base class RenderPass {
       throw StateError('bindTexture called before bindPipeline');
     }
     final unit = pipeline._samplerUnits[slot.uniformName];
-    if (unit == null) return;
+    if (unit == null) {
+      // Match the native backend, which throws when the shader has no sampler
+      // of this name. Swallowing it silently leaves the unit unbound and the
+      // draw samples whatever texture that unit last held, wrong only on web
+      // and varying with draw order. _samplerUnits comes from the shader's
+      // live reflection, not GL introspection, so a reflected-but-unused
+      // sampler still resolves and only a truly absent name throws.
+      throw StateError(
+        'Failed to bind texture. This shader declares no texture named '
+        '"${slot.uniformName}". Check the sampler name spelling, and note '
+        'that a sampler nothing in the shader reads is optimized out by the '
+        'compiler and reflects as absent.',
+      );
+    }
 
     final gl = _gpuContext._gl;
     final target = texture.glTarget;

packages/flutter_scene/lib/src/mesh.dart

@@ -61,17 +61,17 @@ base class Mesh {
   vm.Aabb3? _localBoundsCache;
   bool _localBoundsCached = false;
   List<int>? _cachedBoundsVersions;
+  List<Geometry>? _cachedGeometries;
 
   /// Local-space union of every primitive's [Geometry.localBounds], or
   /// `null` when no primitive has computable bounds.
   ///
-  /// Cached. The cache refreshes itself when a primitive's geometry
-  /// reports a new [Geometry.localBoundsVersion], so an updatable
-  /// geometry that is mutated in place stays correct without an explicit
-  /// invalidation. Call [markLocalBoundsDirty] after replacing a
-  /// primitive's geometry.
+  /// Cached. The cache refreshes itself when a primitive's geometry is
+  /// replaced, or reports a new [Geometry.localBoundsVersion], so swapping
+  /// or mutating a primitive's geometry stays correct without an explicit
+  /// invalidation.
   vm.Aabb3? get localBounds {
-    if (_localBoundsCached && _boundsVersionsUnchanged()) {
+    if (_localBoundsCached && _boundsCacheStillValid()) {
       return _localBoundsCache;
     }
     vm.Aabb3? result;
@@ -89,27 +89,40 @@ base class Mesh {
     _cachedBoundsVersions = <int>[
       for (final p in primitives) p.geometry.localBoundsVersion,
     ];
+    _cachedGeometries = <Geometry>[for (final p in primitives) p.geometry];
     return result;
   }
 
-  bool _boundsVersionsUnchanged() {
-    final snapshot = _cachedBoundsVersions;
-    if (snapshot == null || snapshot.length != primitives.length) {
+  // Valid only if the primitive list still holds the same geometry instances
+  // (identity) at the same bounds versions the cache was built from. The
+  // identity compare is what catches a replaced primitive geometry, whose
+  // fresh instance can share the old one's version number.
+  bool _boundsCacheStillValid() {
+    final versions = _cachedBoundsVersions;
+    final geometries = _cachedGeometries;
+    if (versions == null ||
+        geometries == null ||
+        versions.length != primitives.length) {
       return false;
     }
     for (var i = 0; i < primitives.length; i++) {
-      if (snapshot[i] != primitives[i].geometry.localBoundsVersion) {
+      final geometry = primitives[i].geometry;
+      if (!identical(geometries[i], geometry) ||
+          versions[i] != geometry.localBoundsVersion) {
         return false;
       }
     }
     return true;
   }
 
-  /// Invalidate the cached [localBounds]. Call this after replacing a
-  /// primitive's geometry.
+  /// Invalidate the cached [localBounds]. Rarely needed, since the cache
+  /// already detects a replaced primitive geometry and a bumped
+  /// [Geometry.localBoundsVersion]; call this only after mutating a
+  /// geometry's bounds through a path that leaves its version unchanged.
   void markLocalBoundsDirty() {
     _localBoundsCache = null;
     _localBoundsCached = false;
     _cachedBoundsVersions = null;
+    _cachedGeometries = null;
   }
 }