Page MenuHome

Line Art feature update: List Optimization
ClosedPublic

Authored by YimingWu (NicksBest) on May 19 2021, 9:30 AM.

Details

Summary

Use array instead of ListBase for line art bounding area linked triangles and edges.

Up to 40% increase in performance overall for iterating among triangles in my test scenes. Performance improvements may vary among files and different CPU models.

Diff Detail

Repository
rB Blender

Event Timeline

YimingWu (NicksBest) requested review of this revision.May 19 2021, 9:30 AM
YimingWu (NicksBest) created this revision.
Sebastian Parborg (zeddb) requested changes to this revision.May 19 2021, 7:48 PM

Besides my comments, I think this looks good!

source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
209–210

Hmm, perhaps we could calculate these numbers from the camera settings?

In the new Unreal engine they do something similar where they decimate all the geometry dynamically in the scene so that if a triangle is smaller than a pixel, it is decimated.

This makes it so that they get rid of a lot of useless detail that will not be seen.

Maybe we can do something similar and calculate the smallest tile size?

409

I know the enforcement of not using short and the likes are inconsistent in blender.
But I think we should fix this now so we follow the coding style lines: https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Integer_Types
You can change all the other shorts variables related to lineart as well in this patch so you don't leave any behind, sorry that I didn't catch this sooner.

source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c
319

*rt -> *tri

334

*rl -> *edge

368

rt -> tri_t

This revision now requires changes to proceed.May 19 2021, 7:48 PM
source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
209–210

Then we add a lot of BMesh operation during loading? and the line result will depend on the rendering size, which is likely to produce inconsistent result when rendering small and big, especially for smooth chained lines.

409

OK. I think I'll use the types suggested in the guideline

source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
209–210

I don't mean that we should decimate geometry.

What I mean is that we can calculate the smallest tile that fits in the camera plane.

So you calculate how small a pixel is in image space and then limit the tile size to that.

For example if your render output is 100x100 pixels, then if each tile is split in four every time, then the maximum we would want to split is seven times. (0.78125 x 0.78125 pixels)

But anyways, this was kinda off topic for this patch. So if you do think this is a good idea, then it should be done in an other patch.

YimingWu (NicksBest) marked an inline comment as done.May 20 2021, 2:48 PM
YimingWu (NicksBest) added inline comments.
source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
209–210

Oh actually due to how line art registering cuts in lines, it has to calculate until the very last tiny triangle. Because we can't reduce/simplify triangles that we need to go through, tiling based on pixel size often isn't small enough to let one tile contain only a hundred or so of triangles for better load separation.

source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
209–210

Ah, ok. Then this is irrelevant.

YimingWu (NicksBest) marked 3 inline comments as done.May 24 2021, 8:23 AM
YimingWu (NicksBest) marked an inline comment as done.May 24 2021, 8:47 AM
YimingWu (NicksBest) added inline comments.
source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c
319

I'll fix rt, rl, rv changes separately in a later patch so it don't cause much conflict at the current stage.

Fixed stuff except rt rl etc names.

Fixed stuff except rt rl etc names.

Ok, then I would like a separate cleanup patch were this is fixed as you told me before that you would clean that up.

Updated to reflect on recent naming changes.

YimingWu (NicksBest) marked 3 inline comments as done.

Fixed uncaught naming stuff.

Looks good to me besides my comment.
Fix that and then you can commit.

source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
336

This shouldn't have changed, right?

This revision is now accepted and ready to land.May 27 2021, 2:00 PM
YimingWu (NicksBest) marked an inline comment as done.May 27 2021, 2:08 PM
YimingWu (NicksBest) added inline comments.
source/blender/gpencil_modifiers/intern/lineart/MOD_lineart.h
336

Oh this was a merge mistake. I'll change it back.

YimingWu (NicksBest) marked an inline comment as done.

Fix comment changes.