Page MenuHome

Annotations: Use flag to determine if the layer is a Ruler

Authored by Antonio Vazquez (antoniov) on Wed, Oct 9, 4:02 PM.



Proposed fix for T70141.

Before, the ruler was using the name of the layer as key, but this is very weak because if the layer name changes, the layer gets an annotation layer.

Now, the layer is marked using a flag and now it's possible to rename it.

Diff Detail

rB Blender

Event Timeline

Dalai Felinto (dfelinto) requested changes to this revision.Wed, Oct 9, 5:16 PM

We need a doversion code for this as well, otherwise what happens with old files?



- view3d_ruler_get_layer
+ view3d_ruler_layer_get

We tend to have get/set in the end to group functions together with only their suffix changing and to help IDEs autocomplete. Even if it "sounds wrong" from the English point of view.


No need to spam flag setting into multiple lines, and your comment really is redundant. A good comment would focus more on the "why" not so much on the "what'.

In this case you covered the "what" but it is really redundant, the one line of code says the same thing here.

My suggestion:

- gpl->flag |= GP_LAYER_HIDE;
- /* Enable ruler flag. */
- gpl->flag |= GP_LAYER_IS_RULER;
+ gpl->flag |= GP_LAYER_HIDE | GP_LAYER_IS_RULER;`

Shouldn't this be readonly?

Nitpick, what about is_ruler instead?

This revision now requires changes to proceed.Wed, Oct 9, 5:16 PM
  • Annotations: Fix review problems

Not sure if use this define. I could add the string directly, but as this name is used in other areas I wanted to keep the define to make easier if we need to find where the ruler name is used.

We could move the define into shared place, but not sure where and/or if it worth it.

Some extra suggested changes, but if they get addressed, LGTM


No need to mention the task, git blame will give that already.


doversion can be its own mad house, no need to elegant, shared, ... Honestly, I would probably just add it to the test itself:

- if (STREQ(gpl->info, ruler_name)) {
+ if (STREQ(gpl->info, "RulerData3D")) {

Alternative, to avoid extra nesting by continuing the for loop earlier.

if (gpd == NULL) {

Am I right that can be only one ruler layer? If so I would expect a break; after we found a match.


While we follow "is_*" for rna names, the UI text should be "Ruler" (also change on tooltip):

- RNA_def_property_ui_text(prop, "Is Ruler", "This layer is a special ruler layer");
+ RNA_def_property_ui_text(prop, "Ruler", "This is a special ruler layer");
This revision is now accepted and ready to land.Wed, Oct 9, 8:52 PM
  • Cleanup: Small tweaks