[Bf-committers] [Bf-blender-cvs] SVN commit: /data/svn/bf-blender [35108] trunk/blender: added image-editor operators:

[Brecht Van Lommel]

Hi,

Some comments on the code:

> +static int image_invert_exec(bContext *C, wmOperator *op) {
> +       Image *ima= CTX_data_edit_image(C);
> +       ImBuf *ibuf= BKE_image_get_ibuf(ima, NULL);
> +
> +       // flags indicate if this channel should be inverted
> +       short r,g,b,a;
> +       int i;
> +
> +       r = RNA_boolean_get(op->ptr, "inv_r");
> +       g = RNA_boolean_get(op->ptr, "inv_g");
> +       b = RNA_boolean_get(op->ptr, "inv_b");
> +       a = RNA_boolean_get(op->ptr, "inv_a");
> +
> +       /* TODO: make this into an IMB_invert_channels(ibuf,r,g,b,a) method!? */
> +       if (ibuf->rect_float) {
> +
> +               float *fp = (float *) ibuf->rect_float;
> +               for( i = ibuf->x * ibuf->y; i > 0; i--, fp+=4 ) {
> +                       if( r ) fp[0] = 1.0f - fp[0];
> +                       if( g ) fp[1] = 1.0f - fp[1];
> +                       if( b ) fp[2] = 1.0f - fp[2];
> +                       if( a ) fp[3] = 1.0f - fp[3];
> +               }
> +               IMB_rect_from_float(ibuf);
> +       }
> +       else if(ibuf->rect) {
> +
> +               char *cp = (char *) ibuf->rect;
> +               for( i = ibuf->x * ibuf->y; i > 0; i--, cp+=4 ) {
> +                       if( r ) cp[0] = 255 - cp[0];
> +                       if( g ) cp[1] = 255 - cp[1];
> +                       if( b ) cp[2] = 255 - cp[2];
> +                       if( a ) cp[3] = 255 - cp[3];
> +               }
> +       }
> +       else
> +               return OPERATOR_CANCELLED;
> +
> +       WM_event_add_notifier(C, NC_IMAGE|NA_EDITED, ima);
> +
> +       return OPERATOR_FINISHED;
> +
> +}

The image buffer should be marked as modified: ibuf->userflags |=
IB_BITMAPDIRTY;

> +       RNA_def_boolean(ot->srna, "inv_r", 0, "Red", "Invert Red Channel");
> +       RNA_def_boolean(ot->srna, "inv_g", 0, "Green", "Invert Green Channel");
> +       RNA_def_boolean(ot->srna, "inv_b", 0, "Blue", "Invert Blue Channel");
> +       RNA_def_boolean(ot->srna, "inv_a", 0, "Alpha", "Invert Alpha Channel");

No need to use abbreviations here, change inv_ to invert_.

> @@ -1499,10 +1563,6 @@
>
>        /* flags */
>        ot->flag= OPTYPE_REGISTER|OPTYPE_UNDO;
> -
> -       /* properties */
> -       RNA_def_enum(ot->srna, "method", unpack_method_items, PF_USE_LOCAL, "Method", "How to unpack.");
> -       RNA_def_string(ot->srna, "id", "", 21, "Image Name", "Image datablock name to unpack."); /* XXX, weark!, will fail with library, name collisions */
>  }

Was this intentional, those properties still seem to be used in the
unpack operator?

Brecht.