Masking: refactor mask_rasterize vector copying into a utility function
ClosedPublic
Actions

Authored by Simon Lenz (Amudtogal) on Dec 3 2021, 1:08 PM.

Details

Reviewers

Summary

Add a separate scanline function scanfill_vert_add_v2_with_depth that adds
a vertex from 2D coordinate array & the specified z-value.

Following @Campbell Barton (campbellbarton)'s TODO (mask_rasterize.c:732) this adds a separate scanline function BLI_scanfill_vert2d_add that adds a scanline vertex from 2D coordinate array, and the specified z-value:

This function is then used to directly add the mask vertices to the scanline instead of copying the 2D arrays for each vertex.
BLI_scanfill_vert2d_add(&sf_ctx, co_feather, 1.0f);

Diff Detail

Repository

rB Blender

Branch

scanfill_vert2d (branched from master)

Build Status

Buildable 19179
Build 19179: arc lint + arc unit

Event Timeline

Simon Lenz (Amudtogal) requested review of this revision.Dec 3 2021, 1:08 PM

Simon Lenz (Amudtogal) created this revision.

Harbormaster completed remote builds in B19179: Diff 45608.Dec 3 2021, 1:08 PM

Simon Lenz (Amudtogal) retitled this revision from Reduce copying in mas_rasterize.c to Masking: reduce copy instructions in mask_rasterize.Dec 3 2021, 1:14 PM

Simon Lenz (Amudtogal) edited the summary of this revision. (Show Details)

Simon Lenz (Amudtogal) added a subscriber: Campbell Barton (campbellbarton).

Simon Lenz (Amudtogal) added a reviewer: Campbell Barton (campbellbarton).Dec 3 2021, 1:46 PM

For the most part seems fine, request minor change.

source/blender/blenlib/intern/scanfill.c
151	Having a duplicate function for a slightly more convenient arguments doesn't seem warranted. Suggest to add a static function in `mask_rasterize.c` that copies takes `const float vec[2], const float z` arguments and calls `BLI_scanfill_vert_add`.

This revision now requires changes to proceed.Dec 3 2021, 1:51 PM

I might have missunderstood the intent of the TODO (I thought it was about reducing the two calls of copy_v2_v2 and copy_v3_v3 for each iteration to a single one).

Nevertheless, I moved the vert2d_add version to a static helper function. This still requires creating a temporary 3D vector, which is then passed to the BLI_scanfill_vert_add function.

Is this what you intended?

Harbormaster completed remote builds in B19232: Diff 45694.Dec 6 2021, 10:24 AM

@Campbell Barton (campbellbarton), any updates on the review?

Clang-format, rename BLI_scanfill_vert2d_add to scanfill_vert_add_v2_with_depth, remove redundant declaration.

Harbormaster completed remote builds in B21558: Diff 50298.Apr 10 2022, 9:16 AM

Rename scanfill_vert_add_v2_with_depth arguments (vec,z,vert -> co_xy,co_z,co)

Harbormaster completed remote builds in B21559: Diff 50299.Apr 10 2022, 9:19 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Apr 10 2022, 9:20 AM

Campbell Barton (campbellbarton) retitled this revision from Masking: reduce copy instructions in mask_rasterize to Masking: refactor mask_rasterize vector copying into into a utility function.

Campbell Barton (campbellbarton) accepted this revision.Apr 10 2022, 9:28 AM

Campbell Barton (campbellbarton) retitled this revision from Masking: refactor mask_rasterize vector copying into into a utility function to Masking: refactor mask_rasterize vector copying into a utility function.

Campbell Barton (campbellbarton) added inline comments.