Maniphest T66744

Grease Pencil Build and Simplify Modifier Order Ignored
Closed, ResolvedKNOWN ISSUE
Actions

Assigned To
Antonio Vazquez (antoniov)
Authored By
Philip Holzmann (Foaly)
Jul 12 2019, 10:31 AM
Tags
  • Grease Pencil
  • User Interface
  • BF Blender (2.83)
Subscribers
Antonio Vazquez (antoniov)
Bastien Montagne (mont29)
Brecht Van Lommel (brecht)
Philip Holzmann (Foaly)
Sebastian Parborg (zeddb)

Description

System Information
Operating system: Windows 10 64 bit
Graphics card: Nvidia GTX 750 Ti

Blender Version
Broken: blender-2.80.0-git.d663048696b2-windows64

Short description of error
If a Grease Pencil Object has both a Simplify and Build Modifier, it doesn't matter which order they come in.
It always behaves as if the Simplify Modifier came after the Build Modifier.

I'd expect Simplify before Build to behave as if the Simplify Modifier was applied first.

Exact steps for others to reproduce the error

GreaseBuild.blend581 KBDownload

From left to right:

  1. Simplify before Build
  2. Build before Simplify
  3. Simplify before Build, with Simplify applied

As you can see, 1 and 2 behave the same (when you play the animation).
However, I'd expect 1 to behave the same as 3.

Related Objects

Mentioned Here
rBd0b8b49b447b: GPencil: Use stack order for build modifiers
T66294: GPencil: Move Modifiers evaluation to Depsgraph from Draw Manager
rBd663048696b2: Fix T66691: Ceash trying to render the 2.80 splash image

Event Timeline

Philip Holzmann (Foaly) created this task.Jul 12 2019, 10:31 AM
Sebastian Parborg (zeddb) assigned this task to Antonio Vazquez (antoniov).Jul 12 2019, 11:00 AM
Sebastian Parborg (zeddb) lowered the priority of this task from 90 to 50.
Antonio Vazquez (antoniov) added a comment.Jul 12 2019, 1:52 PM

IIRC this is how is designed because there are several types (internally) of modifiers... modifiers that change geometry, modifiers that generate geometry and modifiers that use time.

I will review, but I think this is a problem of communicate this and explain how and why the modifiers are evaluated in that way.

Antonio Vazquez (antoniov) added a subscriber: Sebastian Parborg (zeddb).EditedJul 12 2019, 4:12 PM

@Sebastian Parborg (zeddb) I have been looking at the code, and the build modifier must be evaluated before the others modifiers due this modifier needs to read all frames to decide what to do, but the other modifiers only read the current frame and strokes.

Run Build before is done for data access and also for performance, so all geometry modifiers (Build, Mirror and Array) run first, and then all modify modifiers. Time modifier plays totatlly different.

I don't think we can change this or consider as a bug, but a limitation of what you can do with modifiers... maybe we could add some type of warning on modifier panel to inform about this.

Note: For 2.81 I'm going to replace where the modifiers are evaluated (see T66294). I have the change done in greasepencil branch, but we decided not to move to 2.80 and wait for 2.81.

Sebastian Parborg (zeddb) added a comment.Jul 12 2019, 4:32 PM

@Antonio Vazquez (antoniov) How about we make it so that the build modifier adds a warning to its UI if it is not first in the stack?

Antonio Vazquez (antoniov) added a comment.Jul 12 2019, 4:40 PM

Yes, this is the idea... I have seen this before in the code for some modifiers, but don't remember where

Antonio Vazquez (antoniov) lowered the priority of this task from 50 to Normal.Jul 18 2019, 10:38 PM
Dalai Felinto (dfelinto) removed Antonio Vazquez (antoniov) as the assignee of this task.Dec 23 2019, 4:33 PM
Dalai Felinto (dfelinto) added a project: Tracker Curfew.
Dalai Felinto (dfelinto) added a subscriber: Antonio Vazquez (antoniov).
Bastien Montagne (mont29) triaged this task as High priority.Jan 27 2020, 4:58 PM
Bastien Montagne (mont29) changed the subtype of this task from "Report" to "Bug".
Bastien Montagne (mont29) edited projects, added Grease Pencil, User Interface, BF Blender (2.83); removed Tracker Curfew, BF Blender.
Bastien Montagne (mont29) added a subscriber: Bastien Montagne (mont29).

That is a fairly sever usability issue, order in the stack should always be actual order of evaluation. If some modifiers need to run first, then they should be forcefully put in first positions of the stack.

We have that with regular Object modifier e.g., Multires cannot be moved or added after a modifier like Build, because the latter breaks access to mesh orig data needed by multiress…

It is most likely too late to address that in 2.82, but would consider it a show-stopper for 2.83 then. You cannot expect users to guess the actual order of evaluation, they have to get what they see here. ;)

Antonio Vazquez (antoniov) added a comment.Jan 27 2020, 5:05 PM

@Bastien Montagne (mont29) Are there any tag to force a modifier to be first in stack?

Bastien Montagne (mont29) added a comment.Jan 27 2020, 5:16 PM

@Antonio Vazquez (antoniov) For 'regular' data check eModifierTypeFlag_RequiresOriginalData flag in BKE_modifier.h. However if you have three different 'stages' you'll probably need another extra tag for the second stage?

Antonio Vazquez (antoniov) claimed this task.Jan 28 2020, 10:44 PM

I'm going to refactor the modifier loop to use the order on the stack. All the changes will be done in greasepencil-refactor branch, because we have changed all modifiers evaluation for the new drawing engine.

Antonio Vazquez (antoniov) added a comment.Jan 28 2020, 10:45 PM

I don't think we must consider as high priority, just a Know Limitation that will be fixed in 2.83 with the new engine and evaluation logic.

Antonio Vazquez (antoniov) added a comment.Jan 28 2020, 11:01 PM

This is fixed in the branch in commit: d0b8b49b447b

Antonio Vazquez (antoniov) lowered the priority of this task from High to Normal.EditedJan 29 2020, 4:54 PM
Antonio Vazquez (antoniov) changed the subtype of this task from "Bug" to "Known Issue".

We keep this as Know Issue in this version because it will be fixed when the refactor branch will be merged in master branch.

Brecht Van Lommel (brecht) moved this task from Backlog to bcon3: Bugs on the BF Blender (2.83) board.Feb 4 2020, 3:17 PM
Brecht Van Lommel (brecht) added a subscriber: Brecht Van Lommel (brecht).Apr 6 2020, 6:12 PM

Is this fixed now?

Antonio Vazquez (antoniov) closed this task as Resolved.Apr 6 2020, 7:07 PM

Yes, fixed in 2.83