[Bf-committers] [Bf-blender-cvs] SVN commit: /data/svn/bf-blender [50446] trunk/blender/source/blender: fix for crash in sequencer introduced with recent cache addition,
Sergey Sharybin
sergey.vfx at gmail.com
Thu Sep 6 10:21:03 CEST 2012
Hi,
I don't think introducing do_cache for external API is actually a nice idea
-- cache should be completely transparent for all areas.
I would say the way to go is:
- Use different functions for usage inside sequencer.c and all other areas.
External areas would use BKE_sequence_free which would ensure cache is in
relevant state for all sequences (same behavior as it was before)
- sequencer.c would internally use quite the same function, which would
invalidate cache for freeing sequence only. Seems it's the correct way of
solving initial problem of invalidating cache when there're sequences with
incorrect list bases (which seems only happening when you're deleting all
the sequences, in this case you wouldn't worry about dependencies -- cache
for all sequences would be freed anyway).
- The only possible reason to expose do_cache flag is usage in RNA API. But
i don't think it'll be noticeable slow if do_cache would be set to TRUE
here, also dependency check could be improved to support finding
dependencies faster than O(N).
On Thu, Sep 6, 2012 at 6:45 AM, Campbell Barton <ideasman42 at gmail.com>wrote:
> Revision: 50446
>
> http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=50446
> Author: campbellbarton
> Date: 2012-09-06 04:45:25 +0000 (Thu, 06 Sep 2012)
> Log Message:
> -----------
> fix for crash in sequencer introduced with recent cache addition,
> - running undo with metastrips would crash immediately.
> - freeing a strip without a scene would crash (clipboard does this).
>
> Modified Paths:
> --------------
> trunk/blender/source/blender/blenkernel/BKE_sequencer.h
> trunk/blender/source/blender/blenkernel/intern/sequencer.c
> trunk/blender/source/blender/blenlib/BLI_path_util.h
> trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c
> trunk/blender/source/blender/editors/space_sequencer/sequencer_edit.c
> trunk/blender/source/blender/makesrna/intern/rna_sequencer_api.c
>
> Modified: trunk/blender/source/blender/blenkernel/BKE_sequencer.h
> ===================================================================
> --- trunk/blender/source/blender/blenkernel/BKE_sequencer.h 2012-09-06
> 03:08:47 UTC (rev 50445)
> +++ trunk/blender/source/blender/blenkernel/BKE_sequencer.h 2012-09-06
> 04:45:25 UTC (rev 50446)
> @@ -199,7 +199,7 @@
>
> void BKE_sequencer_free_clipboard(void);
>
> -void BKE_sequence_free(struct Scene *scene, struct Sequence *seq);
> +void BKE_sequence_free(struct Scene *scene, struct Sequence *seq, const
> int do_cache);
> const char *BKE_sequence_give_name(struct Sequence *seq);
> void BKE_sequence_calc(struct Scene *scene, struct Sequence *seq);
> void BKE_sequence_calc_disp(struct Scene *scene, struct Sequence *seq);
>
> Modified: trunk/blender/source/blender/blenkernel/intern/sequencer.c
> ===================================================================
> --- trunk/blender/source/blender/blenkernel/intern/sequencer.c 2012-09-06
> 03:08:47 UTC (rev 50445)
> +++ trunk/blender/source/blender/blenkernel/intern/sequencer.c 2012-09-06
> 04:45:25 UTC (rev 50446)
> @@ -169,7 +169,7 @@
> MEM_freeN(strip);
> }
>
> -void BKE_sequence_free(Scene *scene, Sequence *seq)
> +void BKE_sequence_free(Scene *scene, Sequence *seq, const int do_cache)
> {
> if (seq->strip)
> seq_free_strip(seq->strip);
> @@ -205,21 +205,32 @@
>
> /* free cached data used by this strip,
> * also invalidate cache for all dependent sequences
> + *
> + * be _very_ careful here, invalidating cache loops over the scene
> sequences and
> + * assumes the listbase is valid for all strips, this may not be
> the case if lists are being freed.
> + * this is optional BKE_sequence_invalidate_cache
> */
> - BKE_sequence_invalidate_cache(scene, seq);
> + if (do_cache) {
> + if (scene) {
> + BKE_sequence_invalidate_cache(scene, seq);
> + }
> + }
>
> MEM_freeN(seq);
> }
>
> +/* cache must be freed before calling this function
> + * since it leaves the seqbase in an invalid state */
> static void seq_free_sequence_recurse(Scene *scene, Sequence *seq)
> {
> - Sequence *iseq;
> + Sequence *iseq, *iseq_next;
>
> - for (iseq = seq->seqbase.first; iseq; iseq = iseq->next) {
> + for (iseq = seq->seqbase.first; iseq; iseq = iseq_next) {
> + iseq_next = iseq->next;
> seq_free_sequence_recurse(scene, iseq);
> }
>
> - BKE_sequence_free(scene, seq);
> + BKE_sequence_free(scene, seq, FALSE);
> }
>
>
> @@ -240,7 +251,7 @@
> seq_free_clipboard_recursive(seq);
> }
>
> - BKE_sequence_free(NULL, seq_parent);
> + BKE_sequence_free(NULL, seq_parent, FALSE);
> }
>
> void BKE_sequencer_free_clipboard(void)
> @@ -269,22 +280,21 @@
> void BKE_sequencer_editing_free(Scene *scene)
> {
> Editing *ed = scene->ed;
> - MetaStack *ms;
> Sequence *seq;
>
> if (ed == NULL)
> return;
>
> + /* this may not be the active scene!, could be smarter about this
> */
> + BKE_sequencer_cache_cleanup();
> +
> SEQ_BEGIN (ed, seq)
> {
> - BKE_sequence_free(scene, seq);
> + BKE_sequence_free(scene, seq, FALSE);
> }
> SEQ_END
>
> - while ((ms = ed->metastack.first)) {
> - BLI_remlink(&ed->metastack, ms);
> - MEM_freeN(ms);
> - }
> + BLI_freelistN(&ed->metastack);
>
> MEM_freeN(ed);
>
>
> Modified: trunk/blender/source/blender/blenlib/BLI_path_util.h
> ===================================================================
> --- trunk/blender/source/blender/blenlib/BLI_path_util.h 2012-09-06
> 03:08:47 UTC (rev 50445)
> +++ trunk/blender/source/blender/blenlib/BLI_path_util.h 2012-09-06
> 04:45:25 UTC (rev 50446)
> @@ -97,7 +97,6 @@
> } bli_rebase_state;
>
> int BLI_rebase_path(char *abs, size_t abs_len, char *rel, size_t rel_len,
> const char *base_dir, const char *src_dir, const char *dest_dir);
> -#define BKE_rebase_path BLI_rebase_path /* remove after a 2012 */
>
> char *BLI_last_slash(const char *string);
> int BLI_add_slash(char *string);
>
> Modified:
> trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c
> ===================================================================
> --- trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c
> 2012-09-06 03:08:47 UTC (rev 50445)
> +++ trunk/blender/source/blender/editors/space_sequencer/sequencer_draw.c
> 2012-09-06 04:45:25 UTC (rev 50446)
> @@ -852,7 +852,7 @@
> struct ImBuf *ibuf = NULL;
> struct ImBuf *scope = NULL;
> struct View2D *v2d = &ar->v2d;
> - int rectx, recty;
> + /* int rectx, recty; */ /* UNUSED */
> float viewrectx, viewrecty;
> float render_size = 0.0;
> float proxy_size = 100.0;
> @@ -874,8 +874,8 @@
> viewrectx = (render_size * (float)scene->r.xsch) / 100.0f;
> viewrecty = (render_size * (float)scene->r.ysch) / 100.0f;
>
> - rectx = viewrectx + 0.5f;
> - recty = viewrecty + 0.5f;
> + /* rectx = viewrectx + 0.5f; */ /* UNUSED */
> + /* recty = viewrecty + 0.5f; */ /* UNUSED */
>
> if (sseq->mainb == SEQ_DRAW_IMG_IMBUF) {
> viewrectx *= scene->r.xasp / scene->r.yasp;
>
> Modified:
> trunk/blender/source/blender/editors/space_sequencer/sequencer_edit.c
> ===================================================================
> --- trunk/blender/source/blender/editors/space_sequencer/sequencer_edit.c
> 2012-09-06 03:08:47 UTC (rev 50445)
> +++ trunk/blender/source/blender/editors/space_sequencer/sequencer_edit.c
> 2012-09-06 04:45:25 UTC (rev 50446)
> @@ -613,7 +613,7 @@
> BLI_remlink(lb, seq);
> if (seq == last_seq)
> BKE_sequencer_active_set(scene, NULL);
> if (seq->type == SEQ_TYPE_META)
> recurs_del_seq_flag(scene, &seq->seqbase, flag, 1);
> - BKE_sequence_free(scene, seq);
> + BKE_sequence_free(scene, seq, TRUE);
> }
> seq = seqn;
> }
> @@ -1812,7 +1812,7 @@
> start_ofs += step;
> }
>
> - BKE_sequence_free(scene, seq);
> + BKE_sequence_free(scene, seq, TRUE);
> seq = seq->next;
> }
> else {
> @@ -2010,7 +2010,7 @@
> last_seq->seqbase.last = NULL;
>
> BLI_remlink(ed->seqbasep, last_seq);
> - BKE_sequence_free(scene, last_seq);
> + BKE_sequence_free(scene, last_seq, TRUE);
>
> /* emtpy meta strip, delete all effects depending on it */
> for (seq = ed->seqbasep->first; seq; seq = seq->next)
>
> Modified: trunk/blender/source/blender/makesrna/intern/rna_sequencer_api.c
> ===================================================================
> --- trunk/blender/source/blender/makesrna/intern/rna_sequencer_api.c
> 2012-09-06 03:08:47 UTC (rev 50445)
> +++ trunk/blender/source/blender/makesrna/intern/rna_sequencer_api.c
> 2012-09-06 04:45:25 UTC (rev 50446)
> @@ -170,7 +170,7 @@
> if (seq->strip->stripdata->name[0] == '\0') {
> BKE_report(reports, RPT_ERROR, "Sequences.new_image:
> unable to open image file");
> BLI_remlink(&ed->seqbase, seq);
> - BKE_sequence_free(scene, seq);
> + BKE_sequence_free(scene, seq, FALSE); /* cache won't have
> been generated */
> return NULL;
> }
>
> @@ -315,7 +315,7 @@
> Scene *scene = (Scene *)id;
>
> BLI_remlink(&ed->seqbase, seq);
> - BKE_sequence_free(scene, seq);
> + BKE_sequence_free(scene, seq, TRUE);
>
> WM_main_add_notifier(NC_SCENE | ND_SEQUENCER, scene);
> }
>
> _______________________________________________
> Bf-blender-cvs mailing list
> Bf-blender-cvs at blender.org
> http://lists.blender.org/mailman/listinfo/bf-blender-cvs
>
--
With best regards, Sergey Sharybin
More information about the Bf-committers
mailing list