This patch adds a new texture mapping mode that maps to the UV space of the stroke curve, tentatively called "roll."
This is a test file for testing the feature on the Draw and Paint brush:
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- temp-sculpt-roll-mapping
- Build Status
Buildable 24523 Build 24523: arc lint + arc unit
Event Timeline
Comment Actions
From the previous meeting notes:
Joe added a temp debug visualizing, which we agreed should stay for the final feature in a simplified form.
A needed addition is to visualize the samples curve already on stroke start.
The stroke curve will also be changed into single color for simplicity.
Comment Actions
- New optimization strategy for spline texture mapping.
- Implemented a quadratic bezier backend. It didn't prove as useful as I thought, will be removed unless there's a use for it.
Comment Actions
Some notes from testing this and reading other users feedback. These are the most important to tackle if we want to commit before bcon2.
- There are still plenty of artifacts, both between the split texture steps, and circular brush falloff artifacts. (The effects are more visible when using sphere or constant falloff)
- I know the current overlay for the stroke was made for debugging. But we should keep using this only while the stroke is not applied yet, like the Stabilize Stroke setting.
By using the same line thickness and color, having both combined should blend nicely together.
After the stroke starts being applied, the overlay should disappear, and only reappear if a new stroke needs to be calculated.
- Enabling the Rake setting in other texture mapping modes is messing up the Roll method.
- Continuing a stroke beyond the boundaries of the mesh will result in issues.
The stroke is never properly concluded up to the boundary.
Once the stroke continues on the mesh the stroke will jump to that point instead of starting anew.
Performance is also quite a lot slower than regular textured strokes. Even a slower stroke does not fully help avoiding this
This should be fixed, or at least optimized after committing to master.
Comment Actions
It's also good too note that there will always be a fade in and out for the textured stroke.
Can this be improved somehow by making sure the strength is applied differently for this method?
Also, there should maybe be something to prevent using this texture method with certain stroke methods. Dots should not be combined with Roll.
Comment Actions
I'd like to review this too, particularly the way the code interacts with the new curve evaluation code. Generally I think the Bezier / even spline code should reuse and improve the existing lower level curve evaluation and parameterization code. I can make that feedback more specific later though.
paint_stroke.c should me moved to C++ in a separate commit (as described in T103343).
Comment Actions
Some quick superficial comments, did not look at this closely yet.
| source/blender/editors/sculpt_paint/paint_stroke.cc | ||
|---|---|---|
| 98 | Can you add a comment explaining the difference between samples and points? | |
| 170 | There is no clear relation between this logic and PAINT_MAX_INPUT_SAMPLES. I think things should be structured in such a way that it's obvious there is enough space in the array. | |
| source/blender/editors/sculpt_paint/sculpt_intern.h | ||
| 562 | This doesn't seem to be used. | |
| 563 | This is set but not read anywhere? | |
Comment Actions
Removing myself as reviewer from sculpt patches since this is not my responsibility and I don't have enough time for it. If it's unclear who should review it can be worked out with Dalai or Thomas.
Comment Actions
@Hans Goudey (HooglyBoogly) You want to still elaborate on your review?
@Joseph Eagar (joeedh) I tested the patch again most issues I mentioned above are still present:
- Continuing a stroke beyond the boundaries of the mesh will result in issues.
The stroke is never properly concluded up to the boundary.
Once the stroke continues on the mesh the stroke will jump to that point instead of starting anew.
Using the Painting brushes also causes a crash now or won't result in any visual changes and quite poor performance.
Comment Actions
@Hans Goudey (HooglyBoogly) You want to still elaborate on your review?
It's really more of a conversation that needs to be had. I feel strongly that we shouldn't add another implementation of curve evaluation and arc length parameterization. Adding a helper class to combine data storage as an abstraction for sculpt mode is reasonable, but the implementation should be shared.
If the existing curve/Bezier code needs to be changed to accommodate that I would absolutely help out.
There are also Brecht's comments and moving the file to C++ separately in master.
Comment Actions
I don't care where the low-level code lives. I did find the existing arc length parameterization code to be pretty unusable.