[Bf-codereview] Automated image sequence offset calculation (issue 5402041)

[Brecht Van Lommel]

Code review seems to be down for maintenance, but here's a review.

It looks ok, except that I think BKE_image_guess_offset should check
for the image source? Maybe it goes ok if ima->name is not set for
generated images, but for movie files it may give issues? For example
with the kind of movie filenames blender animation rendering outputs,
that include frame range numbers.

Brecht.

Also in BKE_image_sequence_first_num, "offset" is an unused variable.

On Wed, Nov 16, 2011 at 8:05 PM,  <sergey.vfx at gmail.com> wrote:
> Reviewers: bf-codereview_blender.org,
>
> Description:
> Automated image sequence offset calculation for image input node and
> image editor.
>
> Calculate offset or sequence when loading image in node or editor and
> also re-calculate offset when switching currently displaying image in
> editor or source image in input node.
>
> This makes using image sequences easier to use when they aren't starting
> from 1 (i've seen several times when artists used external tools to
> rename sequences instead of setting  offset for them)
>
> Also this kind of recalculation is needed because offset is stored in
> ImageUser structure so when you're switching from one sequence to
> another it can be really annoying because offset from previous sequence
> would  be used for new sequence.
>
> Please review this at http://codereview.appspot.com/5402041/
>
> Affected files:
>  source/blender/blenkernel/BKE_image.h
>  source/blender/blenkernel/intern/image.c
>  source/blender/blenkernel/intern/movieclip.c
>  source/blender/editors/space_image/image_ops.c
>  source/blender/makesrna/intern/rna_nodetree.c
>  source/blender/makesrna/intern/rna_space.c
>
>
> Index: source/blender/blenkernel/BKE_image.h
> ===================================================================
> --- source/blender/blenkernel/BKE_image.h (revision 41914)
> +++ source/blender/blenkernel/BKE_image.h (working copy)
> @@ -171,6 +171,9 @@ void BKE_image_merge(struct Image *dest, struct Image
> *source);
>  /* check if texture has alpha (depth=32) */
>  int BKE_image_has_alpha(struct Image *image);
>
> +int BKE_image_sequence_first_num(const char *full_name);
> +void BKE_image_guess_offset(struct Scene *scene, struct Image *ima, struct
> ImageUser *iuser);
> +
>  /* image_gen.c */
>  void BKE_image_buf_fill_color(unsigned char *rect, float *rect_float, int
> width, int height, float color[4]);
>  void BKE_image_buf_fill_checker(unsigned char *rect, float *rect_float, int
> height, int width);
> Index: source/blender/blenkernel/intern/image.c
> ===================================================================
> --- source/blender/blenkernel/intern/image.c (revision 41914)
> +++ source/blender/blenkernel/intern/image.c (working copy)
> @@ -2426,3 +2426,29 @@ int BKE_image_has_alpha(struct Image *image)
>                return 0;
>  }
>
> +int BKE_image_sequence_first_num(const char *full_name)
> +{
> +       unsigned short numlen;
> +       char head[FILE_MAX], tail[FILE_MAX], name[FILE_MAX];
> +       int offset;
> +       char num[FILE_MAX]= {0};
> +
> +       BLI_strncpy(name, full_name, sizeof(name));
> +       BLI_stringdec(name, head, tail, &numlen);
> +
> +       BLI_strncpy(num, full_name+strlen(head), numlen+1);
> +
> +       return atoi(num);
> +}
> +
> +void BKE_image_guess_offset(Scene *scene, Image *ima, ImageUser *iuser)
> +{
> +       int cfra= iuser->framenr-iuser->offset+iuser->sfra;
> +
> +       iuser->offset= BKE_image_sequence_first_num(ima->name)-1;
> +
> +       if(scene)
> +               cfra= scene->r.cfra;
> +
> +       BKE_image_user_calc_frame(iuser, cfra, 0);
> +}
> Index: source/blender/blenkernel/intern/movieclip.c
> ===================================================================
> --- source/blender/blenkernel/intern/movieclip.c (revision 41914)
> +++ source/blender/blenkernel/intern/movieclip.c (working copy)
> @@ -154,7 +154,7 @@ static void get_sequence_fname(MovieClip *clip, int
> framenr, char *name)
>
>        /* movieclips always points to first image from sequence,
>           autoguess offset for now. could be something smarter in the future
> */
> -       offset= sequence_guess_offset(clip->name, strlen(head), numlen);
> +       offset= BKE_image_sequence_first_num(clip->name);
>
>        if (numlen) BLI_stringenc(name, head, tail, numlen,
> offset+framenr-1);
>        else        BLI_strncpy(name, clip->name, sizeof(clip->name));
> Index: source/blender/editors/space_image/image_ops.c
> ===================================================================
> --- source/blender/editors/space_image/image_ops.c (revision 41914)
> +++ source/blender/editors/space_image/image_ops.c (working copy)
> @@ -817,8 +817,9 @@ static int image_open_exec(bContext *C, wmOperator *op)
>        /* initialize because of new image */
>        if(iuser) {
>                iuser->sfra= 1;
> -               iuser->offset= 0;
>                iuser->fie_ima= 2;
> +
> +               BKE_image_guess_offset(NULL, ima, iuser);
>        }
>
>        /* XXX unpackImage frees image buffers */
> Index: source/blender/makesrna/intern/rna_nodetree.c
> ===================================================================
> --- source/blender/makesrna/intern/rna_nodetree.c (revision 41914)
> +++ source/blender/makesrna/intern/rna_nodetree.c (working copy)
> @@ -479,6 +479,17 @@ static EnumPropertyItem
> *renderresult_layers_add_enum(RenderLayer *rl)
>        return item;
>  }
>
> +static void rna_Node_image_update(Main *bmain, Scene *scene, PointerRNA
> *ptr)
> +{
> +       bNode *node= (bNode*)ptr->data;
> +       Image *ima= (Image *)node->id;
> +       ImageUser *iuser= node->storage;
> +
> +       BKE_image_guess_offset(scene, ima, iuser);
> +
> +       rna_Node_update(bmain, scene, ptr);
> +}
> +
>  static EnumPropertyItem *rna_Node_image_layer_itemf(bContext *UNUSED(C),
> PointerRNA *ptr, PropertyRNA *UNUSED(prop), int *free)
>  {
>        bNode *node= (bNode*)ptr->data;
> @@ -1624,7 +1635,7 @@ static void def_cmp_image(StructRNA *srna)
>        RNA_def_property_struct_type(prop, "Image");
>        RNA_def_property_flag(prop, PROP_EDITABLE);
>        RNA_def_property_ui_text(prop, "Image", "");
> -       RNA_def_property_update(prop, NC_NODE|NA_EDITED, "rna_Node_update");
> +       RNA_def_property_update(prop, NC_NODE|NA_EDITED,
> "rna_Node_image_update");
>
>        RNA_def_struct_sdna_from(srna, "ImageUser", "storage");
>
> Index: source/blender/makesrna/intern/rna_space.c
> ===================================================================
> --- source/blender/makesrna/intern/rna_space.c (revision 41914)
> +++ source/blender/makesrna/intern/rna_space.c (working copy)
> @@ -120,6 +120,7 @@ EnumPropertyItem viewport_shade_items[] = {
>  #include "BKE_colortools.h"
>  #include "BKE_context.h"
>  #include "BKE_depsgraph.h"
> +#include "BKE_image.h"
>  #include "BKE_paint.h"
>  #include "BKE_scene.h"
>  #include "BKE_screen.h"
> @@ -513,8 +514,11 @@ static void rna_SpaceImageEditor_image_set(PointerRNA
> *ptr, PointerRNA value)
>  {
>        SpaceImage *sima= (SpaceImage*)(ptr->data);
>        bScreen *sc= (bScreen*)ptr->id.data;
> +       Scene *scene= sc->scene;
> +       Image *ima= (Image*)value.data;
>
> -       ED_space_image_set(sima, sc->scene, sc->scene->obedit,
> (Image*)value.data);
> +       ED_space_image_set(sima, scene, scene->obedit, ima);
> +       BKE_image_guess_offset(scene, ima, &sima->iuser);
>  }
>
>  static EnumPropertyItem *rna_SpaceImageEditor_draw_channels_itemf(bContext
> *UNUSED(C), PointerRNA *ptr,
>
>
>