Page MenuHome
Differential D14588

Initialize ATTR_SURFACE_TRIANGLE_INDEX
ClosedPublic
Actions

Authored by Angus Stanton (abstanton) on Apr 7 2022, 4:08 PM.

Details

Reviewers
Jacques Lucke (JacquesLucke)
Hans Goudey (HooglyBoogly)
Maniphest Tasks
T97138: Initialize newly created curve attributes correctly.
Commits
rBa45976957e00: Curves: correctly initialize new attributes
Summary

If ATTR_SURFACE_TRIANGLE_INDEX does not already exist,
fill it with -1 on getting for write.

Diff Detail

Repository
rB Blender
Branch
T97138-initialize-curve-attributes (branched from master)
Build Status
Buildable 21512
Build 21512: arc lint + arc unit

Event Timeline

Angus Stanton (abstanton) requested review of this revision.Apr 7 2022, 4:08 PM
Angus Stanton (abstanton) created this revision.
Harbormaster completed remote builds in B21512: Diff 50212.Apr 7 2022, 4:08 PM
Angus Stanton (abstanton) added a reviewer: Jacques Lucke (JacquesLucke).Apr 7 2022, 4:11 PM
Jacques Lucke (JacquesLucke) requested changes to this revision.Apr 7 2022, 6:14 PM

Thanks for the patch. I think this should be generalized a bit, i.e. the expected default value should be passed to get_mutable_attribute.

This revision now requires changes to proceed.Apr 7 2022, 6:14 PM
Jacques Lucke (JacquesLucke) added a reviewer: Hans Goudey (HooglyBoogly).Apr 7 2022, 6:15 PM
Angus Stanton (abstanton) added a comment.Apr 8 2022, 2:27 AM
In D14588#391597, @Jacques Lucke (JacquesLucke) wrote:

Thanks for the patch. I think this should be generalized a bit, i.e. the expected default value should be passed to get_mutable_attribute.

Shall I overload get_mutable_attribute with a new function that takes a default value, or shall I change the existing implementation and change all it's call sites?

Angus Stanton (abstanton) updated this revision to Diff 50234.EditedApr 8 2022, 2:58 AM
  • Overload get_mutable_attribute to accept default value

Thought I'd go with overloading for now, can convert all function calls to use the default value accepting function if needed

Harbormaster completed remote builds in B21521: Diff 50234.Apr 8 2022, 2:58 AM
Jacques Lucke (JacquesLucke) added a comment.Apr 8 2022, 10:35 AM

Think it would be better to always pass a default value to get_mutable_attribute. get_mutable_attribute could be smart enough to not initialize new attribute layers twice in case the default value is zero. That can also be handled separately though.

Angus Stanton (abstanton) updated this revision to Diff 50263.Apr 8 2022, 6:24 PM
  • Use default value for all uses of get_mutable_attribute
Harbormaster completed remote builds in B21537: Diff 50263.Apr 8 2022, 6:24 PM
Jacques Lucke (JacquesLucke) accepted this revision.Apr 9 2022, 10:44 AM
This revision is now accepted and ready to land.Apr 9 2022, 10:44 AM
Closed by commit rBa45976957e00: Curves: correctly initialize new attributes (authored by Angus Stanton (abstanton), committed by Jacques Lucke (JacquesLucke)). · Explain WhyApr 9 2022, 10:54 AM
This revision was automatically updated to reflect the committed changes.
Jacques Lucke (JacquesLucke) added a commit: rBa45976957e00: Curves: correctly initialize new attributes.

Revision Contents
Changeset List

PathSize
source/
blender/
blenkernel/
intern/
13 lines

Diff 50212

View Options

source/blender/blenkernel/intern/curves_geometry.cc

Loading...