Improvements to change main histogram to 65535 bins #5904
Conversation
|
Sample shot of RAW histogram: Same shot, but now the log-linear regular histogram: |
|
The new histogram looks much more useful. One think I immediately noticed is that the green histogram covers the red, and the blue covers red and green. Would it be possible to compose the individual histograms to make all of them more visible? |
|
@Lawrence37 What do you mean exactly by 'to compose'? Put the separate RGB histograms next to each other, or on top of each other, instead of overlapping? I'm not sure that would be very useful considering the relatively small scale of the histogram... This is one of the points that are currently less than optimal. It will probably only really improve if there is a better way to connect the points, instead of drawing lines from the axis. For that I need a way to skip empty bins or something smarter, like dynamic binning depending on the width of the histogram. |
|
An alternative to opacity is to introduce a pattern instead of a solid fill. |
|
I mean blending the histograms. darktable, for example, uses additive blending. Some people find it confusing though. Using the old line style avoids the issue, but I have a feeling that it's much harder to implement than it sounds. |
|
@afr-e What kind of pattern would you propose? Side-note: currently, drawing 65536 lines per histogram is slow, but I believe that drawing a polygon and filling it, is virtually unworkable (the L channel used to use a filling mode, but it took 20 seconds when you enabled it, and then it didn't draw anything). |
|
@Lawrence37 I experimented once with additive blending here, but there was some opposition. |
I am willing to help on this |
|
@Thanatomanic Can you point me to the code which causes the dead slowdown? |
|
Excellent Roel, I like the top log-log raw histogram, I can clearly see if/how much specular clipping there is (a little bit), where reflected exposure actually begins (less than a stop from clipping) and if/how many blocked pixels there are (a few). What are the units of the two axes? Ideally gradation every stop, with 1/3EV just in the highlights, some indication of where mid-gray could be (EV0, -3EV from clipping?). I personally do not mind a toothy histogram, though you could widen the bars in the deep shadows if you felt that was more readable. There is a lot of information there so readability is important even when the histogram is small, I find it a bit hard to see the blocked/clipped histogram 'bars' against the dark background. One of the linearX-logY histogram's main uses for me is to nail down how close To The Right reflected Exposure ended up, so it would be useful to have gradation every thousand or two on the X axis. I personally practically never use the linear-linear histogram view. As for vertical log axis units, If we take something of the order of 0.01% as a typical acceptable number of clipped/blocked pixels, the axis should show clearly whether a few, a few hundred, thousand or tens of thousand pixels are clipped/blocked. I see that RD shows gradation at 1, 10, 50, 200, 1k, 5k, 20k, 100k, 500k etc., not bad. As for speed, the raw histogram(s) needs to be computed once only, and it doesn't necessarily need to be the first thing to come up. Anyways, just one user's usage, I hope others will contribute theirs. Thanks a lot for doing this! Jack |
|
@Thanatomanic diff --git a/rtgui/histogrampanel.cc b/rtgui/histogrampanel.cc
index 6331e8364..77d13670d 100644
--- a/rtgui/histogrampanel.cc
+++ b/rtgui/histogrampanel.cc
@@ -25,6 +25,9 @@
#include "../rtengine/LUT.h"
#include "rtimage.h"
#include "../rtengine/color.h"
+#include "../rtengine/sleef.h"
+#define BENCHMARK
+#include "../rtengine/StopWatch.h"
using namespace rtengine;
@@ -365,10 +368,16 @@ void HistogramPanel::toggleButtonMode ()
//
//
// HistogramScaling
-double HistogramScaling::log(double vsize, double val)
+float HistogramScaling::log(float vsize, float val) const
{
//double factor = 10.0; // can be tuned if necessary - higher is flatter curve
- return vsize * std::log(factor / (factor + val)) / std::log(factor / (factor + vsize));
+ return vsize * xlogf(factor / (factor + val)) / xlogf(factor / (factor + vsize));
+}
+
+float HistogramScaling::log_test(float vsize, float val, float mult) const
+{
+ //double factor = 10.0; // can be tuned if necessary - higher is flatter curve
+ return vsize * xlogf(factor / (factor + val)) * mult;
}
//
@@ -958,22 +967,25 @@ void HistogramArea::on_realize ()
void HistogramArea::drawCurve(Cairo::RefPtr<Cairo::Context> &cr,
const LUTu & data, double scale, int hsize, int vsize)
{
+ BENCHFUNMICRO
double s = RTScalable::getScale();
cr->set_line_width(s);
//cr->move_to (padding, vsize - 1);
scale = scale <= 0.0 ? 0.001 : scale; // avoid division by zero and negative values
+ const float temp1 = 1.f / xlogf(HistogramScaling::factor / (HistogramScaling::factor + vsize));
+ const float temp2 = 1.f / xlogf(HistogramScaling::factor / (HistogramScaling::factor + 65535.0));
for (int i = 0; i < 65536; i++) {
double val = data[i] * (double)vsize / scale;
if (drawMode > 0) { // scale y for single and double log-scale
- val = HistogramScaling::log ((double)vsize, val);
+ val = HistogramScaling::log_test (vsize, val, temp1);
}
double iscaled = i;
if (drawMode == 2) { // scale x for double log-scale
- iscaled = HistogramScaling::log (65535.0, (double)i);
+ iscaled = HistogramScaling::log_test (65535.f, i, temp2);
}
double posX = padding + iscaled * (hsize - padding * 2.0) / 65535.0;
diff --git a/rtgui/histogrampanel.h b/rtgui/histogrampanel.h
index 740b0a12c..497a0c3aa 100644
--- a/rtgui/histogrampanel.h
+++ b/rtgui/histogrampanel.h
@@ -49,9 +49,10 @@ struct HistogramRGBAreaIdleHelper {
class HistogramScaling
{
public:
- double factor;
- HistogramScaling() : factor(10.0) {}
- double log (double vsize, double val);
+ float factor;
+ HistogramScaling() : factor(10.f) {}
+ float log (float vsize, float val) const;
+ float log_test (float vsize, float val, float mult) const;
};
class HistogramRGBArea final : public Gtk::DrawingArea, public BackBuffer, private HistogramScaling, public rtengine::NonCopyable
|
|
@Thanatomanic |
I was thinking of the classic Morse code for lines, and stippling and hatching for area under the curve, a different pattern for each channel. However, care must be taken so as not to disturb or distract the user. General thoughts Histograms are meant to help the user glean as much useful info as possible. In my opinion, that can (and often should) mean that you don't necessarily have to represent the data exactly; i.e. it should be represented symbolically and emphasize important elements. E.g. the outlier lines are not only difficult to read because of the dark background as @jackho9 noted but because they are thin. Simply putting a dot on top of those lines and highlighting them (persistently or on hover) would make them more apparent and recognizable. I tend to be brimming with ideas, so feel free to ask me for more feedback. 😉 |
|
@Thanatomanic With the SSE code for the log mode I'm now at ~5ms per color from which ~4.5ms are for drawing the lines. I think, that's fine now. Interestingly, latest clang auto vectorizes the two loops, but latest gcc does not... |
|
@heckflosse Thank you, these speed ups make the performance return to how it was before the change. Only when you show the RAW histogram for huge files (100 MP), and drag to change the factor, you have a very slow response. But that's perfectly fine for me. |
I do not understand why the MP count of a file has an influence on the response time when dragging. Can you elaborate on this? |
Yes, thank you. Question: is the factor necessary? |
|
@heckflosse I've timed both @jackho9 I think I implemented the flexible factor a few years back to have some form of 'zoom' functionality in the histogram and allow you to choose which area to look for details. Maybe that is no longer necessary because the histogram now actually renders as it should and more details are visible anyhow. As for the axes: the x-axis divisions need some work, fully agreed on that. I'll see what I can do for a proper EV scale. The more pressing matter for me is to get a nicer non-raw histogram that looks smoother than it does now. And I just got an idea on how to do that 😄 |
|
One thing that I suggested for PhotoFlow that may be applicable here is that it would be nice to have labels for the axes and lines.
Not wild. At the very least, if there is an indication of the clipping on the graph, I would be happy. |
|
@heckflosse I think that It does not explain why the drawing scales with the resolution of the image. What does factor in, is what @Lawrence37 mentioned before:
So, the more the lines need drawing, the slower it gets. A histogram with a lot of variation would be slower than a flat one. That still doesn't quite explain why the time scales with megapixels... |
|
Would it have to do with the calculations that generate the histogram? |
ImageIO::getTIFFSampleFormat |
|
This pull request introduces 2 alerts when merging a11dffd into d02ed52 - view on LGTM.com new alerts:
|
|
This pull request introduces 2 alerts when merging 65ff20a into d02ed52 - view on LGTM.com new alerts:
|
|
@heckflosse @Lawrence37 @afr-e @Desmis @Beep6581 Finally I have made good progress today, up to the point that I believe this is ready for finetuning, testing and a merge. Your feedback would be much appreciated, in particular on whether you like the look or how it could be improved. Biggest changes since the last time I worked on this, is that the histogram now knows what the bit depth of the raw image is. The gridlines and histogram are adjusted accordingly. You can easily check the difference by comparing raw images with e.g. 12 bit and 16 bit. The normal histogram is always 8 bit and is now scaled properly again (it was stretched into 65536 bins before...). Thanks for taking a look! |
|
I just spotted that the RGB readout bar is not working in all cases. I'll take a look in a few days when I have some time again. |
|
This pull request introduces 2 alerts when merging 2e58bae into d02ed52 - view on LGTM.com new alerts:
|
|
Some observations:
|
|
|
@Thanatomanic |
|
@Lawrence37 The primary reason for not using the lines was that prior to my bitdepth changes, there could be a lot of empty bins, making the line go up and down a lot and messing up the visualization. I will try out how it will look now. Good point about the sRGB middle gray. |
|
Another idea is to only plot points. The down side is that they will be hard to see on the left side because they are spaced apart. |
|
I have tried to re-introduce the old line mode and it works well for the normal histogram, but for the raw histogram things look pretty terrible:
The problem is that the really raw sensor data are rescaled according to the black and white levels. Because the histogram has integer bins, this leads to quantization errors. I see three possible ways out of this:
I think the 2nd option is the nicest, but also the most work (I think). The 1st option is not very useful, except to analyse the actual sensor data. The 3rd option is easy to implement but the quantization errors remain. Thoughts? Perhaps I should pose this question on Pixls for some wider feedback. |
|
I don't quite understand what the second option means so I apologize if this is the same as your idea. My approach would be this: |
|
So, let's assume you have a 16 bit file, with a black point of 512 and a white point of 65400. Currently we subtract 512 from all sensor values and then rescale so that (65400 - 512) becomes 65535. How would your approach differ? |
|
After black point subtraction, the binning occurs. There are 64889 bins that may have values (bins number 0 to 64888) so we plot a line using 64889 points. On the linear histogram, bin 0 is on the left edge and bin 64888 is on the right edge. On the log histogram, bin 64888 is on the right edge. Bin 32444 is on the first EV line from the right. |
That makes it very useful to me. I need to see two things in my work:
|
|
Converting this to a draft because the way the histogram is drawn is up for discussion. |
Following the discussion here (#5899), I made a branch
histogram-improvedto work on implementing a more accurate main histogram. The initial commit simply upscales the number of bins naively and spreads out values over all bins. The three main consequences of this:About these visual artifacts. For example, the histogram of the working image is constructed from an 8-bit
Image8source. I now simply spread out the 265 pixel values over all 65536 bins, so many bins remain empty. This leaves a lot of empty room between lines. I already changed the drawing mode to single vertical lines, instead of connected lines, but I am still thinking about how this could be improved.Expect more changes in the upcoming week(s). Suggestions are highly appreciated.