Regression: Mesh deformed through keyframes does not update normals on frame change #120594

New Issue

Closed

opened 2024-04-12 20:28:49 +02:00 by Damien Picard · 9 comments

Damien Picard commented

2024-04-12 20:28:49 +02:00

Member

System Information
Operating system: Linux-6.5.0-27-generic-x86_64-with-glibc2.38 64 Bits, X11 UI
Graphics card: NVIDIA GeForce RTX 3060 Laptop GPU/PCIe/SSE2 NVIDIA Corporation 4.6.0 NVIDIA 545.29.06

Blender Version
Broken: version: 4.2.0 Alpha, branch: main, commit date: 2024-04-09 17:06, hash: 20bc1cbb5792
Broken: version: 4.1.0, branch: blender-v4.1-release, commit date: 2024-03-25 20:42, hash: 40a5e739e270
Worked: version: 3.6.8, branch: blender-v3.6-release, commit date: 2024-01-15 12:55, hash: f6ae60f3eeda

Short description of error
When mesh vertex positions are animated directly through f-curves, the normals are not recalculated on each frame change, resulting in incoherent normals.

Exact steps for others to reproduce the error

Open attached blend file.
Drag current frame to 1 in Timeline editor.
The normals are still those from the last frame, even though the positions have changed.
Switch to Edit Mode and back to Object Mode.
The normals are now fixed.

On file open	After frame change	After mode toggle

This type of manual mesh animation can be achieved through the AnimAll add-on.

Originally reported by @wanigamo in blender/blender-addons#105279

**System Information** Operating system: Linux-6.5.0-27-generic-x86_64-with-glibc2.38 64 Bits, X11 UI Graphics card: NVIDIA GeForce RTX 3060 Laptop GPU/PCIe/SSE2 NVIDIA Corporation 4.6.0 NVIDIA 545.29.06 **Blender Version** Broken: version: 4.2.0 Alpha, branch: main, commit date: 2024-04-09 17:06, hash: `20bc1cbb5792` Broken: version: 4.1.0, branch: blender-v4.1-release, commit date: 2024-03-25 20:42, hash: `40a5e739e270` Worked: version: 3.6.8, branch: blender-v3.6-release, commit date: 2024-01-15 12:55, hash: `f6ae60f3eeda` **Short description of error** When mesh vertex positions are animated directly through f-curves, the normals are not recalculated on each frame change, resulting in incoherent normals. **Exact steps for others to reproduce the error** 1. Open attached blend file. 2. Drag current frame to 1 in Timeline editor. The normals are still those from the last frame, even though the positions have changed. 3. Switch to Edit Mode and back to Object Mode. The normals are now fixed. | On file open | After frame change | After mode toggle | | -------- | -------- | -------- | | ![image](/attachments/ca9a3c40-3a14-4702-abab-16404abf0f36) | ![image](/attachments/3cf0fa6b-e550-45c0-ade1-acc0ac60a625) | ![image](/attachments/69dc962a-6455-464c-8cde-52a0a18d280a) | This type of manual mesh animation can be achieved through the AnimAll add-on. Originally reported by @wanigamo in blender/blender-addons#105279

animall.blend

106 KiB

69 KiB

63 KiB

64 KiB

Damien Picard added the

labels 2024-04-12 20:28:50 +02:00

Damien Picard referenced this issue from blender/blender-addons

2024-04-12 20:31:16 +02:00

AnimAll/ Terrible lag and wrong face-normal after insert Points Location key on mesh object #105279

Iliya Katushenock added the

label 2024-04-12 20:31:44 +02:00

Iliya Katushenock changed title from ~~Mesh deformed through keyframes does not update normals on frame change~~ to Regression: Mesh deformed through keyframes does not update normals on frame change

2024-04-12 20:31:52 +02:00

Iliya Katushenock commented

2024-04-14 11:40:30 +02:00

Member

Please, describe how you animate vertices.

Please, describe how you animate vertices.

Iliya Katushenock added

Needs Information from User

and removed

labels 2024-04-14 11:40:39 +02:00

Damien Picard commented

2024-04-14 15:38:30 +02:00

Author

Member

AnimAll allows you to insert keyframes on vertex positions and other attributes. You can go to the Animation tab in the viewport, then enable Position in the AnimAll panel and switch to Edit Mode. Then click Insert Key, move a few frames, move the vertex, and Insert Key again.

You could also run this script and then you don’t need AnimAll at all:

import bpy

bpy.ops.mesh.primitive_cube_add()

bpy.context.scene.frame_end = 10

bpy.context.scene.frame_set(1)
bpy.context.object.data.vertices[0].co.x = 0
bpy.context.object.data.keyframe_insert("vertices[0].co", index=0)

bpy.context.scene.frame_set(10)
bpy.context.object.data.vertices[0].co.x = -5
bpy.context.object.data.keyframe_insert("vertices[0].co", index=0)

AnimAll allows you to insert keyframes on vertex positions and other attributes. You can go to the Animation tab in the viewport, then enable Position in the AnimAll panel and switch to Edit Mode. Then click Insert Key, move a few frames, move the vertex, and Insert Key again. You could also run this script and then you don’t need AnimAll at all: ```python import bpy bpy.ops.mesh.primitive_cube_add() bpy.context.scene.frame_end = 10 bpy.context.scene.frame_set(1) bpy.context.object.data.vertices[0].co.x = 0 bpy.context.object.data.keyframe_insert("vertices[0].co", index=0) bpy.context.scene.frame_set(10) bpy.context.object.data.vertices[0].co.x = -5 bpy.context.object.data.keyframe_insert("vertices[0].co", index=0) ```

Iliya Katushenock added

Animation & Rigging

and removed

Needs Information from User

labels 2024-04-14 15:40:42 +02:00

Iliya Katushenock commented

2024-04-14 15:41:37 +02:00

Member

Not sure if this is correct to do this..

Not sure if this is correct to do this..

Iliya Katushenock added

and removed

labels 2024-04-14 15:43:03 +02:00

Iliya Katushenock commented

2024-04-14 15:43:09 +02:00

Member

@HooglyBoogly /

@HooglyBoogly /

Damien Picard commented

2024-04-14 15:46:32 +02:00

Author

Member

Not sure if this is correct to do this..

Not sure either, but it used to be supported and I think manual mesh animation is still very useful in some cases.

If it’s no longer supported I guess I’ll add a handler in AnimAll to force recalculation of normals on frame change but that’s not optimal.

> Not sure if this is correct to do this.. Not sure either, but it used to be supported and I think manual mesh animation is still very useful in some cases. If it’s no longer supported I guess I’ll add a handler in AnimAll to force recalculation of normals on frame change but that’s not optimal.

Hans Goudey commented

2024-04-15 20:56:28 +02:00

Member

I don't think this was every supported really. Normals were not cached on original objects for years until recent changes. RNA setters never cleared the cached normals.

I can easily make this work for animating MeshVertex.co. But unfortunately we have no system in place to invalidate caches from the RNA attribute API. And the right approach isn't obvious, because inserting cache invalidation into every single set operation is sure to impact performance significantly.

Maybe the right solution is this:

Add cache invalidation (mesh->tag_positions_changed();) to rna_MeshVertex_co_set
Add another function tag_positions_changed() for changes to the attribute API.

OTOH, maybe skipping step 2 is fine for now. Maybe we'll get a better system for the attribute API eventually anyway.

@JacquesLucke Any thoughts about this?

I don't think this was every supported really. Normals were not cached on original objects for years until recent changes. RNA setters never cleared the cached normals. I can easily make this work for animating `MeshVertex.co`. But unfortunately we have no system in place to invalidate caches from the RNA attribute API. And the right approach isn't obvious, because inserting cache invalidation into every single set operation is sure to impact performance significantly. Maybe the right solution is this: 1. Add cache invalidation (`mesh->tag_positions_changed();`) to `rna_MeshVertex_co_set` 2. Add another function `tag_positions_changed()` for changes to the attribute API. OTOH, maybe skipping step 2 is fine for now. Maybe we'll get a better system for the attribute API eventually anyway. @JacquesLucke Any thoughts about this?

Hans Goudey added

and removed

labels 2024-04-15 20:56:46 +02:00

Jacques Lucke commented

2024-04-16 12:36:09 +02:00

Member

I'm quite surprised that this keyframe works (didn't try). But I also don't see a fundamental reason for why it should be forbidden.

Add cache invalidation (mesh->tag_positions_changed();) to rna_MeshVertex_co_set

That seems reasonable. This shouldn't really affect performance if we remove the tag from rna_Mesh_update_positions_tag, right?

We'll have to think about how to handle this in the rna attribute api soonish too. Think this is related to #118507.

I'm quite surprised that this keyframe works (didn't try). But I also don't see a fundamental reason for why it should be forbidden. > Add cache invalidation (`mesh->tag_positions_changed();`) to `rna_MeshVertex_co_set` That seems reasonable. This shouldn't really affect performance if we remove the tag from `rna_Mesh_update_positions_tag`, right? We'll have to think about how to handle this in the rna attribute api soonish too. Think this is related to #118507.

Iliya Katushenock commented

2024-04-16 12:38:42 +02:00

Member

Ah, animation update can't tag mesh as updated because this will be happen in just one dependence graph evaluation...

Ah, animation update can't tag mesh as updated because this will be happen in just one dependence graph evaluation...

Hans Goudey commented

2024-04-17 14:29:45 +02:00

Member

That seems reasonable. This shouldn't really affect performance if we remove the tag from rna_Mesh_update_positions_tag, right?

Since this property already has a setter, I think not. But it's not a general solution for existing RNA properties without setters, since adding them disables important raw array access optimizations.

I'll add that update tag for now to keep this working, but the problem of the animation system not calling the update functions has come up before. Maybe we need some in between callback that can't "do anything" like the update function, but isn't called when setting every element in an array like the set function.

Or maybe attribute arrays should actually be array properties rather than collections. Anyway...

>That seems reasonable. This shouldn't really affect performance if we remove the tag from `rna_Mesh_update_positions_tag`, right? Since this property already has a setter, I think not. But it's not a general solution for existing RNA properties without setters, since adding them disables important raw array access optimizations. I'll add that update tag for now to keep this working, but the problem of the animation system not calling the update functions has come up before. Maybe we need some in between callback that can't "do anything" like the update function, but isn't called when setting every element in an array like the set function. Or maybe attribute arrays should actually be array properties rather than collections. Anyway...

Hans Goudey referenced this issue from a commit

2024-04-17 14:43:52 +02:00

Fix #120594: Mesh vertex position change in RNA missing update tag

Hans Goudey closed this issue

2024-04-17 14:43:53 +02:00

Blender Bot added

and removed

labels 2024-04-17 14:43:54 +02:00

Sign in to join this conversation.

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

4 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#120594

No description provided.