hey,
i put all the dubious patches in a local branch here:
55d1c92
0c37ddd
31078c9
6697fa5
4e8b208
4d80fef
6d5b72b
6fcecc7
e68c272
89aae89
157cabb
67c7855
632d76d
f22f041
8611850
22e44ab
d1dd982
3dd24f6
bea8972
8aa6adb
5f46f0a
164ad8c
branching off from before 699: 86f353f98d58c7940a09b432f5c4bd76df41b6d8
and put one more patch on top (cherry-picked to master now, too). the net
diff of the whole adventure is now:
$ git diff pre-699-merge..699
diff --git a/src/develop/develop.c b/src/develop/develop.c
index 6207277..fd4d055 100644
--- a/src/develop/develop.c
+++ b/src/develop/develop.c
@@ -275,13 +275,6 @@ void dt_dev_process_image_job(dt_develop_t *dev)
dt_mipmap_cache_read_get(darktable.mipmap_cache, &buf, dev->
image_storage.id, DT_MIPMAP_FULL, DT_MIPMAP_BLOCKING);
dt_show_times(&start, "[dev]", "to load the image.");
- // copy over image now that width and height are sure to be correct:
- const dt_image_t *img = dt_image_cache_read_get(darktable.image_cache,
dev->image_storage.id);
- dev->image_storage = *img;
- // but don't lock the real thing, as that would avoid any writers to
change stuff.
- // (such as raw loading or star rating changes)
- dt_image_cache_read_release(darktable.image_cache, img);
-
// failed to load raw?
if(!buf.buf)
{
@@ -392,12 +385,27 @@ restart:
dt_pthread_mutex_unlock(&dev->pipe_mutex);
}
-void dt_dev_reload_image(dt_develop_t *dev, const uint32_t imgid)
+// load the raw and get the new image struct, blocking in gui thread
+static inline void _dt_dev_load_raw(dt_develop_t *dev, const uint32_t
imgid)
{
+ // first load the raw, to make sure dt_image_t will contain all and
correct data.
+ dt_mipmap_buffer_t buf;
+ dt_times_t start;
+ dt_get_times(&start);
+ dt_mipmap_cache_read_get(darktable.mipmap_cache, &buf, imgid,
DT_MIPMAP_FULL, DT_MIPMAP_BLOCKING);
+ dt_mipmap_cache_read_release(darktable.mipmap_cache, &buf);
+ dt_show_times(&start, "[dev]", "to load the image.");
+
const dt_image_t *image = dt_image_cache_read_get(darktable.image_cache,
imgid);
dev->image_storage = *image;
dt_image_cache_read_release(darktable.image_cache, image);
+}
+
+void dt_dev_reload_image(dt_develop_t *dev, const uint32_t imgid)
+{
+ _dt_dev_load_raw(dev, imgid);
dev->image_force_reload = dev->image_loading = dev->preview_loading = 1;
+
dev->pipe->changed |= DT_DEV_PIPE_SYNCH;
dt_dev_invalidate(dev); // only invalidate image, preview will follow
once it's loaded.
}
@@ -434,9 +442,8 @@ float dt_dev_get_zoom_scale(dt_develop_t *dev,
dt_dev_zoom_t zoom, int closeup_f
void dt_dev_load_image(dt_develop_t *dev, const uint32_t imgid)
{
- const dt_image_t *image = dt_image_cache_read_get(darktable.image_cache,
imgid);
- dev->image_storage = *image;
- dt_image_cache_read_release(darktable.image_cache, image);
+ _dt_dev_load_raw(dev, imgid);
+
if(dev->pipe)
{
dev->pipe->processed_width = 0;
which i would consider not dangerous. this should still fix the problems
with race conditions on dt_image_t, but leave the colorin filmstrip bug
broken for now.
let me know if that addresses your concerns or whether you want that diff
above to be reverted, too.
cheers,
jo
On Fri, Oct 24, 2014 at 9:48 AM, johannes hanika <hana...@gmail.com> wrote:
> hi all.
>
> could you two tell me how to reproduce the lens correction/crash issues? i
> pretty much reverted everything reload_defaults related already. the only
> thing that should be in master (and could stay imho, don't see how it would
> cause problems) is the blocking loading of the full buffer via rawspeed to
> init all the dt_image_t special data.
>
> i'll carefully review all related patches and the net diff and see if
> there is a broken line that slipped, and revert that too.
>
> and yes. in the long term i vote for a rewrite of some parts here:
> - module instance initialisation is messy and different to regular modules
> - maybe we don't need the preview pipeline any more if raw loading is as
> fast as it is?
> - reload_defaults is funny. we should carefully rethink that and maybe opt
> for another interface (like: separate gui and core in modules in a way that
> they can live on their own, so we can just call module_core->init() and
> leave module_gui->init() as it was when switching images?)
>
> cheers,
> jo
>
>
> On Thu, Oct 23, 2014 at 10:33 PM, Pedro Côrte-Real <pe...@pedrocr.net>
> wrote:
>
>> On Thu, Oct 23, 2014 at 9:15 PM, Ulrich Pegelow
>> <ulrich.pege...@tongareva.de> wrote:
>> > My impression: we either shift 1.6 by at least 2 months until everything
>> > is sorted out, or we urgently need to revert the latest commits to bring
>> > us back to a working state :(
>>
>> Yeah, unless hanatos has an idea on how to fix things we should
>> probably revert. The code seems to be extremely brittle. Current
>> master is crashing for me.
>>
>> We probably need to forget about this for 1.6 and just plan a partial
>> rewrite of this right after.
>>
>> Pedro
>>
>
>
------------------------------------------------------------------------------
_______________________________________________
darktable-devel mailing list
darktable-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/darktable-devel
