PhotoFlow: new caching mechanism - TESTING NEEDED!

paulmiller · October 9, 2019, 1:23pm

Assign Profile: working
Spot/Area WB mode: working

When I check the settings, the tile cache size is set to 10,000 not 1,000.

Carmelo_DrRaw · October 9, 2019, 2:11pm

I just fixed this as well. Thanks for checking!

Next step is to re-introduce the histogram visualization, after which I think the code might be good to be committed into the stable branch.

gadolf · October 9, 2019, 5:34pm

(To avoid the same mistake as before, I’m running new-caching-24343 from 2019/10/08)

Crash when trying to open the country sunset play raw:

Some notes:

I have already a .pfi file in the folder from a previous edit, but I chose to open the .CR2 file;
A couple of days ago I clean up my / partition because it was getting full. I did a couple of packages uninstalls, although I don’t remember having uninstalled some of the build required packages. But I always feel uneasy to do sudo apt autoremove (i had problems with this once in the past)
Follows gdb output last lines and log.

RawImage::RawImage() finished
… layer “RAW loader” finished
Building layer “RAW developer”…
RawPreprocessorPar::init_wb_coefficients() called
… layer “RAW developer” finished
PF::Image::do_update(): pipeline #0 updated.
PF::Image::do_update(): preparing pipeline #1
PF::Image::do_update(): updating pipeline #1
PF::new_operation: creating operation of type “raw_loader”
PF::new_operation: creating operation of type “blender”
PF::new_operation: creating operation of type “uniform”
PF::new_operation: creating operation of type “raw_developer_v2”
PF::new_operation: creating operation of type “blender”
PF::new_operation: creating operation of type “uniform”
Building layer “RAW loader”…
… layer “RAW loader” finished
Building layer “RAW developer”…
RawPreprocessorPar::init_wb_coefficients() called
OpParBase::build_many_internal(): adding tilecache for output image #0, padding=0
phf_tile_pool_init(): called
OpParBase::build_many_internal(): added tilecache for output image #0, padding=0
… layer “RAW developer” finished
[New Thread 0x7fffda7fc700 (LWP 13340)]
PF::Image::do_update(): pipeline #1 updated.
PF::Image::do_update(): preparing pipeline #2
Pipeline: force_rebuild set to true
PF::Image::do_update(): updating pipeline #2
PF::new_operation: creating operation of type “raw_loader”
PF::new_operation: creating operation of type “blender”
PF::new_operation: creating operation of type “uniform”
PF::new_operation: creating operation of type “raw_developer_v2”
PF::new_operation: creating operation of type “blender”
PF::new_operation: creating operation of type “uniform”
Building layer “RAW loader”…
… layer “RAW loader” finished
Building layer “RAW developer”…
RawPreprocessorPar::init_wb_coefficients() called
… layer “RAW developer” finished
PF::Image::do_update(): pipeline #2 updated.
IMAGE_REBUILD finished
Pipeline: force_rebuild set to true
IMAGE_REBUILD started
PF::Image::do_update(): preparing pipeline #0
PF::Image::do_update(): updating pipeline #0
PF::Image::do_update(): pipeline #0 updated.
PF::Image::do_update(): preparing pipeline #1
PF::Image::do_update(): updating pipeline #1
Building layer “RAW loader”…
… layer “RAW loader” finished
Building layer “RAW developer”…
RawPreprocessorPar::init_wb_coefficients() called
OpParBase::build_many_internal(): adding tilecache for output image #0, padding=0
phf_tile_pool_init(): called
OpParBase::build_many_internal(): added tilecache for output image #0, padding=0
… layer “RAW developer” finished
PF::Image::do_update(): pipeline #1 updated.
PF::Image::do_update(): preparing pipeline #2
PF::Image::do_update(): updating pipeline #2
PF::Image::do_update(): pipeline #2 updated.
IMAGE_REBUILD finished
IMAGE_REDRAW_START started
IMAGE_REDRAW_START finished
IMAGE_REDRAW started
[New Thread 0x7fffdb7fe700 (LWP 13341)]
[New Thread 0x7fffdbfff700 (LWP 13342)]
[Thread 0x7fffdb7fe700 (LWP 13341) exited]
[Thread 0x7fffdbfff700 (LWP 13342) exited]
[New Thread 0x7fffd9ffb700 (LWP 13343)]
[Thread 0x7fffd9ffb700 (LWP 13343) exited]
[New Thread 0x7fffbffff700 (LWP 13344)]
[Thread 0x7fffbffff700 (LWP 13344) exited]
[New Thread 0x7fffbffff700 (LWP 13345)]
[New Thread 0x7fffd9ffb700 (LWP 13346)]
[New Thread 0x7fffdbfff700 (LWP 13347)]
[New Thread 0x7fffdb7fe700 (LWP 13348)]

Thread 29 “worker” received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffbffff700 (LWP 13345)]
0x00007ffff697f645 in g_mutex_lock ()
from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
(gdb) set pagination off
(gdb) set logging file log.txt
(gdb) set logging on
Copying output to log.txt.
(gdb) info threads

log.txt (46.9 KB)

Carmelo_DrRaw · October 9, 2019, 7:42pm

There is a problem with the tile allocation. In the log I only see

phf_tile_pool_init(): called

without any further message afterwards. What is the tile cache size in your settings?

This means I need to better handle the errors in the tile cache initialization…
Meanwhile I have added some more messages to better understand the source of the problem.

gadolf · October 9, 2019, 9:38pm

I’ve built it and this time I didn’t have any issues and could complete an edit until the end.

It was 1 GB and I changed it to 2 GB before starting this edit.

paulmiller · October 10, 2019, 11:05am

It’ll be great to get this and the simplified-pipeline changes into stable because together they make Photoflow much nicer to use (more responsive and less likely to have crashes because I forget to set the input of a layer correctly).

If you are looking at the histogram code, maybe it is worth looking at the scaling on the histogram display at the same time? Currently, if you have a lot of clipped area in the design (so the histogram has a large peak), the rest of the histogram gets vertically compressed to invisibility (e.g. as you increase exposure, the histogram collapses to a flat line). I would prefer to be able to see the detail in the rest of the histogram rather than the full height of the relatively uninteresting clipped data peak.

gadolf · October 10, 2019, 12:04pm

I got one more crash.
This time, after doing many, many edits, inserting and deleting heavy layers (edge sharpening and noise reduction presets). Until this point, everything was just fine.
The edit was almost done and I was just trying to fine tune NR, by adding a working channel mixer layer just to spot which channel was introducing more noise, and then tweak the NR tool against that channel.
More specifically, the crash happened when I pulled the red channel value down to zero. I intended to get the blue channel at 100%, but then the crash happened.

IMAGE_REBUILD finished
IMAGE_REDRAW_START started
IMAGE_REDRAW_START finished
IMAGE_REDRAW started
[New Thread 0x7fffd8d20700 (LWP 22384)]
[New Thread 0x7fffd9990700 (LWP 22385)]
[New Thread 0x7fffbd7fe700 (LWP 22386)]
[New Thread 0x7fffdbfff700 (LWP 22387)]
[Thread 0x7fffd9990700 (LWP 22385) exited]
[Thread 0x7fffbd7fe700 (LWP 22386) exited]
[Thread 0x7fffd8d20700 (LWP 22384) exited]
[Thread 0x7fffdbfff700 (LWP 22387) exited]
[New Thread 0x7fffdbfff700 (LWP 22388)]
[New Thread 0x7fffbd7fe700 (LWP 22389)]
[Thread 0x7fffdbfff700 (LWP 22388) exited]
[Thread 0x7fffbd7fe700 (LWP 22389) exited]
[New Thread 0x7fffd9990700 (LWP 22390)]
[Thread 0x7fffd9990700 (LWP 22390) exited]
[New Thread 0x7fffd8d20700 (LWP 22391)]
[Thread 0x7fffd8d20700 (LWP 22391) exited]
IMAGE_REDRAW finished
IMAGE_REDRAW_END started
IMAGE_REDRAW_END finished
OperationsTreeDialog::on_button_clicked: id=1
OperationsTreeDialog::add_layer(): page=1
OperationsTreeDialog::add_layer(): w=0x555559086678  tw=0x555559086678
OperationsTreeDialog::add_layer(): op_tree=0x555559086720
OperationsTreeDialog::add_layer(): image=0x555558c22230
OperationsTreeDialog::add_layer(): layer=0x555560c48f00
Adding layer of type "Channel Mixer" (channel_mixer)
PF::new_operation_with_gui: creating operation of type "channel_mixer"
PF::new_operation: creating operation of type "channel_mixer"
PF::new_operation: creating operation of type "uniform"
IMAGE_REBUILD started
PF::Image::do_update(): preparing pipeline #0
PF::Image::do_update(): updating pipeline #0
PF::new_operation: creating operation of type "channel_mixer"
PF::new_operation: creating operation of type "blender"
PF::new_operation: creating operation of type "uniform"
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #0 updated.
PF::Image::do_update(): preparing pipeline #1
PF::Image::do_update(): updating pipeline #1
PF::new_operation: creating operation of type "channel_mixer"
PF::new_operation: creating operation of type "blender"
PF::new_operation: creating operation of type "uniform"
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #1 updated.
PF::Image::do_update(): preparing pipeline #2
PF::Image::do_update(): updating pipeline #2
PF::new_operation: creating operation of type "channel_mixer"
PF::new_operation: creating operation of type "blender"
PF::new_operation: creating operation of type "uniform"
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #2 updated.
IMAGE_REBUILD finished

(photoflow:4368): Gdk-WARNING **: 08:50:14.991: gdk_window_set_icon_list: icons too large
IMAGE_REBUILD started
PF::Image::do_update(): preparing pipeline #0
PF::Image::do_update(): updating pipeline #0
PF::Image::do_update(): pipeline #0 updated.
PF::Image::do_update(): preparing pipeline #1
PF::Image::do_update(): updating pipeline #1
PF::Image::do_update(): pipeline #1 updated.
PF::Image::do_update(): preparing pipeline #2
PF::Image::do_update(): updating pipeline #2
PF::Image::do_update(): pipeline #2 updated.
IMAGE_REBUILD finished
IMAGE_REDRAW_START started
IMAGE_REDRAW_START finished
IMAGE_REDRAW started
[New Thread 0x7fffd8d20700 (LWP 22401)]
[New Thread 0x7fffd9990700 (LWP 22402)]
[New Thread 0x7fffbd7fe700 (LWP 22403)]
[New Thread 0x7fffdbfff700 (LWP 22404)]
[Thread 0x7fffd9990700 (LWP 22402) exited]
[Thread 0x7fffdbfff700 (LWP 22404) exited]
[Thread 0x7fffbd7fe700 (LWP 22403) exited]
[Thread 0x7fffd8d20700 (LWP 22401) exited]
[New Thread 0x7fffdbfff700 (LWP 22405)]
[New Thread 0x7fffbd7fe700 (LWP 22406)]
[New Thread 0x7fffd9990700 (LWP 22407)]
[New Thread 0x7fffd8d20700 (LWP 22408)]
[Thread 0x7fffbd7fe700 (LWP 22406) exited]
[Thread 0x7fffdbfff700 (LWP 22405) exited]
[Thread 0x7fffd9990700 (LWP 22407) exited]
[Thread 0x7fffd8d20700 (LWP 22408) exited]
IMAGE_REDRAW finished
IMAGE_REDRAW_END started
IMAGE_REDRAW_END finished
IMAGE_REBUILD started
PF::Image::do_update(): preparing pipeline #0
PF::Image::do_update(): updating pipeline #0
Building layer "relight"...
... layer "relight" finished
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #0 updated.
PF::Image::do_update(): preparing pipeline #1
PF::Image::do_update(): updating pipeline #1
Building layer "relight"...
OpParBase::build_many_internal(): adding tilecache for output image #0, padding=0
OpParBase::build_many_internal(): added tilecache for output image #0, padding=0
... layer "relight" finished
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #1 updated.
PF::Image::do_update(): preparing pipeline #2
PF::Image::do_update(): updating pipeline #2
Building layer "relight"...
... layer "relight" finished
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #2 updated.
IMAGE_REBUILD finished
OperationConfigGUI::disable_editing(): setting editing flag to false
IMAGE_REBUILD started
PF::Image::do_update(): preparing pipeline #0
PF::Image::do_update(): updating pipeline #0
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #0 updated.
PF::Image::do_update(): preparing pipeline #1
PF::Image::do_update(): updating pipeline #1
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #1 updated.
PF::Image::do_update(): preparing pipeline #2
PF::Image::do_update(): updating pipeline #2
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #2 updated.
IMAGE_REBUILD finished
IMAGE_REDRAW_START started
IMAGE_REDRAW_START finished
IMAGE_REDRAW started
[New Thread 0x7fffd8d20700 (LWP 22411)]
[New Thread 0x7fffd9990700 (LWP 22412)]
[New Thread 0x7fffbd7fe700 (LWP 22413)]
[New Thread 0x7fffdbfff700 (LWP 22414)]
[Thread 0x7fffd8d20700 (LWP 22411) exited]
[Thread 0x7fffbd7fe700 (LWP 22413) exited]
[Thread 0x7fffdbfff700 (LWP 22414) exited]
[Thread 0x7fffd9990700 (LWP 22412) exited]
[New Thread 0x7fffdbfff700 (LWP 22415)]
[New Thread 0x7fffbd7fe700 (LWP 22416)]
[New Thread 0x7fffd9990700 (LWP 22417)]
[New Thread 0x7fffd8d20700 (LWP 22418)]
[Thread 0x7fffbd7fe700 (LWP 22416) exited]
[Thread 0x7fffd8d20700 (LWP 22418) exited]
[Thread 0x7fffdbfff700 (LWP 22415) exited]
[Thread 0x7fffd9990700 (LWP 22417) exited]
IMAGE_REDRAW finished
IMAGE_REDRAW_END started
IMAGE_REDRAW_END finished
IMAGE_REBUILD started
PF::Image::do_update(): preparing pipeline #0
PF::Image::do_update(): updating pipeline #0
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #0 updated.
PF::Image::do_update(): preparing pipeline #1
PF::Image::do_update(): updating pipeline #1
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #1 updated.
PF::Image::do_update(): preparing pipeline #2
PF::Image::do_update(): updating pipeline #2
Building layer "channel mixer"...
... layer "channel mixer" finished
PF::Image::do_update(): pipeline #2 updated.
IMAGE_REBUILD finished
IMAGE_REDRAW_START started
IMAGE_REDRAW_START finished
IMAGE_REDRAW started
[New Thread 0x7fffd8d20700 (LWP 22427)]
[New Thread 0x7fffd9990700 (LWP 22428)]
[New Thread 0x7fffbd7fe700 (LWP 22429)]
[New Thread 0x7fffdbfff700 (LWP 22430)]
[Thread 0x7fffd9990700 (LWP 22428) exited]
[Thread 0x7fffd8d20700 (LWP 22427) exited]
[Thread 0x7fffdbfff700 (LWP 22430) exited]
[Thread 0x7fffbd7fe700 (LWP 22429) exited]
[New Thread 0x7fffdbfff700 (LWP 22431)]
[New Thread 0x7fffbd7fe700 (LWP 22432)]
[New Thread 0x7fffd9990700 (LWP 22433)]

Thread 16548 "worker" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffdbfff700 (LWP 22431)]
0x0000555555d93ec9 in LUT<float>::operator[]<float, void> (
    index=-nan(0x400000), this=0x555559117000)
    at /home/gustavo/PhotoFlow/src/base/../rt/rtengine/LUT.h:449
449	        int idx = (int)index;  // don't use floor! The difference in negative space is no problems here
(gdb) set pagination off
(gdb) set logging file log.txt
(gdb) set logging on
Copying output to log.txt.
(gdb) info threads
  Id   Target Id         Frame 
  1    Thread 0x7ffff7fada00 (LWP 4368) "photoflow" 0x00007ffff2f199e2 in malloc

log.txt (40.2 KB)

EDIT: Forgot to mention that soon after adding the channel mixer layer, I activated a relight layer that was already there in the pipe, to help me “see” better the noise. Maybe it didn’t liked getting a pure black image (when I set the red channel down to zero), but this is just a thought.

EDIT2: Please disregard my previous comment. I opened the last pfi I had, deactivated all layers, added a channel mixer layer then set reds as zero and it crashed. It seems this has nothing to do with the new caching mechanism, right?
EDIT 3: +1 on @paulmiller’s suggestion to scale the histogram

Carmelo_DrRaw · October 10, 2019, 2:13pm

I am trying to reproduce it, not luck so far. However, from your gdb backtrace I see there was some NAN used as index for a LUT in one ICC conversion. I am trying to figure out how this could happen.

Concerning the visualization of specific RGB channels, another, more convenient way to do that is to add a “clone” layer, select the layer for which you want to inspect the channels, and set the “source channel” accordingly:

18%20PM

@paulmiller I totally agree concerning the histogram, will do that as soon as the basic functionality is back into operation in the new-caching branch.

gadolf · October 10, 2019, 2:17pm

To put things simpler, please, try this:

open a raw file
add channel mixer
change reds to 0 (all channels should be zero at this point)

I repeated this procedure with a raw file I’ve never edited before with phf and it crashed again.

Thanks for the tip!

Carmelo_DrRaw · October 10, 2019, 2:19pm

Exactly what I did, but the program could handle the black image just fine… I’ll investigate further later today.

Carmelo_DrRaw · October 11, 2019, 1:45pm

@gadolf @paulmiller I have just finished implementing some further optimizations of the pipeline, which now allow to also update the histogram efficiently.

New packages should be available in a short time.

Concerning the histogram, how would you suggest to represent the portion >1?
One possibility coud be to have three histogram display modes:

linear, showing values between 0 and 1
perceptual, again showing values between 0 and 1
log2, in which case I would opt for a 10-stops window that can slide on the left and on the right, so that one can focus either on shadows or highlights. The default would be to show a range from 2^-10 to 1.

What do you think?

gadolf · October 11, 2019, 2:06pm

Still on the new caching branch?

I liked that, with three different display modes.
Even allowing to scroll the histogram to the left or right, I would add some indicator to the histogram frame informing that there’s more data to the left or right. I can only recall CHDK histogram with its upper dots (red/orange), indicating under or over exposure:

Carmelo_DrRaw · October 11, 2019, 2:12pm

Yes, I will merge the changes into stable once I will get some more feedback regarding stability. I have introduced once more some non-trivial changes in the pipeline, so better to be cautious…

gadolf · October 11, 2019, 2:51pm

@Carmelo_DrRaw Is it this one?

Carmelo_DrRaw · October 11, 2019, 2:54pm

@gadolf yes, that one.
I forgot to mention that I have also fixed the problem with the channel mixer, which was definitely generating NAN values when all sliders were at zero.

paulmiller · October 11, 2019, 3:04pm

7adc6 seems to be working fine after some brief testing - the histogram is certainly responding well.

There is a performance issue with Samplers though (I did some testing and its in the previous version as well, so not to do with the Histogram changes). If you have Samplers active then the UI thread is blocked when you make changes to layer visiblility (you click a checkbox and nothing happens for a few seconds - it looks like the interface is frozen). Adjusting the settings on a layer does not have this problem.
e.g with:
performance.pfi (41.5 KB)
(the associated raw file is the one from the recent Rice Field Play Raw).
Add some Samplers (I used 4), and then try turning the layer named “TOGGLE ME” on and off.

Histograms:
Is the current behaviour is to show a histogram with bins evently distributed between between 0 and 1 in the current colour space (so you get a linear histogram unless you change the colour space to e.g. sRGB)?
And the vertical scale is adjusted to fit the highest peak?

I think that in an unbounded workflow (no clipping), you probably want to be able to see the full range of data somehow, so an adjustable upper limit is a good idea (probably with a ‘1.0’ marker on the histogram so you can see where clipping would take place). I don’t find histograms on linear data very useful (but thats probably just what I am used to).

Questions I’m trying to answer from looking at the histogram:

am I clipping data?
is there anything there I can recover?
am I using the output range appropriately? (Its quite easy to end up with an image which only uses 75% of the available range on the screen if you don’t have a visible white reference)
is the output brightness appropriate?

Being able to view a histogram of an area of the image instead of the whole thing is also useful.

Carmelo_DrRaw · October 11, 2019, 3:35pm

I will have a look, thanks for the pfi file!

Yes. Currently the histogram shows the values in the colorspace that comes out of the pipeline.

Me neither, and I often add a colorspace conversion layer just to have a perceptual representation. That’s why I would like to decouple the encoding of the image data and the horizontal scale of the histogram…

Currently this can be already achieved by cropping the image. The histogram will then show only the values in the cropped area.

A possible option would be to add a checkbox that toggles between “clipped” and “full” ranges, with markers showing “0” and “1” in the full-range case. Plus a way to choose between linear, perceptual and log2 scales.

afre · October 11, 2019, 5:01pm

Great to see so many items (finally) being addressed. One reminder is to add labels to the axes of the histogram and the curve graphs to give them context.

gadolf · October 11, 2019, 8:58pm

I’m editing an image from scratch and so far so good (around 17 layers).
In the beginning, I felt a delay of around 10 seconds each time I added a layer (the ones I used: relight, tone mapping, curves), but I tried adding more layers just now and there’s no more delay.
Also, the modules sliders present a noticeable stuttering, they don’t move in a fluid way.
Anyway, it’s stable so far.

Carmelo_DrRaw · October 11, 2019, 9:10pm

I have just committed a fix for the UI freezing, it should be much more responsive now when toggling the layer visibility.

A short explanation of why the samplers are slow on this image.
Samplers use an average over a portion of 7x7 pixels to estimate the image color, so they should be very fast to compute. However, if the layer stack contains tools that involve a convolution with some large radius (like the local contrast), then few pixels above this layer become many more pixels below it, due to the required padding. This is mitigated through pixel caching. But when a layer below a large convolution is changed, all downstream caches must be dropped and re-computed…

A suggestion concerning tools positioning: I strongly suggest to put all layers that change the geometry at the top of the stack, for two reasons:

they are usually rather slow, and you don’t want the to be the input of large convolutions
if you introduce opacity masks, you want them to be independent of the geometric tweaks (perpective, rotation, scaling, cropping) that you apply. By putting all layer masks below the geometry adjustments you do not have to worry if you change a rotation or a perspective correction, the masks will still be doing exactly the same because they act on the original image geometry.

In the specific case of the performance.pfi file, this is how I would arrange the layers:

Coool!

I guess this is due to the initial caching. Both relight and local contrast involve quite large convolutions.

That’s true, mostly because for the moment I opted for a safe but sub-optimal thread synchronization between the UI and the processing pipeline. I think there is still room for improvement, but I am giving priority to stability for now…

I went through a period where I was focusing on new tools, and I could not force me to spend even 5 minutes on usability and UI improvements . Now the motivation is somehow back!

Absolutely!

Thanks to all!