Tweak to histogram.c: A proposal

pigeonhill

1155

Tweak to histogram.c: A proposal

« on: 31 / January / 2020, 01:44:29 »

Advertisements

The current histogram has an under and over indicator, that is nearly impossible to see: certainly for those with older eyes and/or in difficult lighting conditions.

The indicators are simply too small.

My request/proposal is that we increase the indicator size from the current 5 to 7.

As I don't compile I can't test if this is 'good enough' or not, eg do we need to go to 9?

Anyway, assuming going from 5 to 7 is better, here is my proposed change to histogram.c

Logged

pigeonhill

1155

Re: Tweak to histogram.c: A proposal

« Reply #1 on: 31 / January / 2020, 12:31:48 »

Oops
X and Y !!!!!

Logged

reyalp

14080

Re: Tweak to histogram.c: A proposal

« Reply #2 on: 03 / February / 2020, 21:41:48 »

Can you please post any proposed change as a diff, attached to the post? Screenshots are really not helpful way to do this.

Also, I moved this thread to "CHDK Development / General Discussion and Assistance". This is the preferred place for discussion of CHDK code changes.

« Last Edit: 04 / February / 2020, 00:14:17 by reyalp »

Logged

Don't forget what the H stands for.

pigeonhill

1155

Re: Tweak to histogram.c: A proposal

« Reply #3 on: 04 / February / 2020, 00:11:57 »

@reyalp

Sorry for only supplying the screen shot.

Here is source and the diff on my github.

https://gist.github.com/pigeonhill/128d25c00f6edecbd8b1b2c089fa650d/revisions

Logged

Caefix

945
Sorry, busy deleting test shots...

Re: Tweak to histogram.c: A proposal

« Reply #4 on: 15 / January / 2022, 16:01:02 »

One if more to avoid some flicker ...

Code: [Select]

//../histogram ~~497~~
   } else {
                if (histo_magnification>105) //  :)
                draw_rectangle(conf.histo_pos.x, conf.histo_pos.y-FONT_HEIGHT, conf.histo_pos.x+8*FONT_WIDTH, conf.histo_pos.y-1, MAKE_COLOR(COLOR_TRANSPARENT, COLOR_TRANSPARENT), RECT_BORDER0|DRAW_FILLED);
            }

Logged

All lifetime is a loan from eternity.

philmoz

3450

Re: Tweak to histogram.c: A proposal

« Reply #5 on: 15 / January / 2022, 16:49:33 »

Quote from: Caefix on 15 / January / 2022, 16:01:02

One if more to avoid some flicker ...
Code: [Select]
//../histogram ~~497~~ } else { if (histo_magnification>105) // draw_rectangle(conf.histo_pos.x, conf.histo_pos.y-FONT_HEIGHT, conf.histo_pos.x+8*FONT_WIDTH, conf.histo_pos.y-1, MAKE_COLOR(COLOR_TRANSPARENT, COLOR_TRANSPARENT), RECT_BORDER0|DRAW_FILLED); }

Did you even bother to test this?

In the 'else' block you modified 'histogram_magnification' will ALWAYS be 0 so your new if statement will always be false and draw_rectangle will never be called. The result is the magnification factor never gets erased after it has been drawn at least once.

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

Caefix

945
Sorry, busy deleting test shots...

Re: Tweak to histogram.c: A proposal

« Reply #6 on: 17 / January / 2022, 11:13:18 »

Quote from: philmoz on 15 / January / 2022, 16:49:33

Quote from: Caefix on 15 / January / 2022, 16:01:02
One if more to avoid some flicker ...

Sorry, didn´t recognize, that I deleted factor by toggeling <Alt>mode...

Code: [Select]

// ~~60~~
static long histo_mag_delete=0;
// ~~491~~
        if (conf.histo_auto_ajust){
            if (histo_magnification>110) {  // :)
				histo_mag_delete=1;
                char osd_buf[64];
                sprintf(osd_buf, " %d.%02dx ", histo_magnification/100, histo_magnification%100);
                draw_string(conf.histo_pos.x, conf.histo_pos.y-FONT_HEIGHT, osd_buf, hc);
            } else if (is_osd_edit){
                draw_string(conf.histo_pos.x, conf.histo_pos.y-FONT_HEIGHT, " 9.99x ", hc);
            } else {
				if (histo_mag_delete)  // :)
                draw_rectangle(conf.histo_pos.x, conf.histo_pos.y-FONT_HEIGHT, conf.histo_pos.x+8*FONT_WIDTH, conf.histo_pos.y-1, MAKE_COLOR(COLOR_TRANSPARENT, COLOR_TRANSPARENT), RECT_BORDER0|DRAW_FILLED);
				histo_mag_delete=0;
            }
        }

Logged

All lifetime is a loan from eternity.

Caefix

945
Sorry, busy deleting test shots...

Re: Tweak to histogram.c: A proposal

« Reply #7 on: 25 / January / 2022, 14:41:41 »

Had expected both lines drawing same, but the shorter draws the longer ...

Code: [Select]

    //Vertical lines
  //  if (conf.histo_show_ev_grid) for (i=1;i<=4;i++) draw_line(x+(1+HISTO_WIDTH)*i/5, y, x+(1+HISTO_WIDTH)*i/5, y+HISTO_HEIGHT, FG_COLOR(hc2));

    if (conf.histo_show_ev_grid) for (i=1;i<=4;i++) draw_vline(x+(1+HISTO_WIDTH)*i/5, y, y+HISTO_HEIGHT, FG_COLOR(hc2));

&& another fine line ...

Code: [Select]

	if (!camera_info.state.mode_play) draw_hline(HISTO_WIDTH-10, conf.histo_pos.y+HISTO_HEIGHT-(shooting_get_tv96()/16+20), 12, COLOR_WHITE);
    }
}

// =========  MODULE INIT =================

&& ptp & LV not quite congruent...

(compression)

Code: [Select]

/platform/generic/wrappers.c ~~1638
// Y multiplier for cameras with 480 pixel high viewports (CHDK code assumes 240)
int __attribute__((weak)) vid_get_viewport_yscale() {
	return 1;               // For most cameras viewport is 240 pixels high
}

Digic6 defaults to 1, maybe some code expects 480 & return 2; together ?

Edit2: Compiled same 6055 with

Quote

#ifndef THUMB_FW
return 1; // For most cameras viewport is 240 pixels high
#else
return 2;
#endif

,

now the draw_vline() is not too long any more ....

Sx270chdkptp_20220125_histo.jpg (81.49 kB, 960x240 - viewed 12 times.)

« Last Edit: 25 / January / 2022, 15:34:33 by Caefix »

Logged

All lifetime is a loan from eternity.

philmoz

3450

Re: Tweak to histogram.c: A proposal

« Reply #8 on: 25 / January / 2022, 17:23:02 »

Quote from: Caefix on 25 / January / 2022, 14:41:41

Had expected both lines drawing same, but the shorter draws the longer ...
Code: [Select]
//Vertical lines // if (conf.histo_show_ev_grid) for (i=1;i<=4;i++) draw_line(x+(1+HISTO_WIDTH)*i/5, y, x+(1+HISTO_WIDTH)*i/5, y+HISTO_HEIGHT, FG_COLOR(hc2)); if (conf.histo_show_ev_grid) for (i=1;i<=4;i++) draw_vline(x+(1+HISTO_WIDTH)*i/5, y, y+HISTO_HEIGHT, FG_COLOR(hc2));

What is the 3rd parameter to draw_vline? Hint it is NOT a Y co-ordinate.

Quote

&& another fine line ...
Code: [Select]
if (!camera_info.state.mode_play) draw_hline(HISTO_WIDTH-10, conf.histo_pos.y+HISTO_HEIGHT-(shooting_get_tv96()/16+20), 12, COLOR_WHITE); } } // ========= MODULE INIT =================

What is the point of drawing a line based on the shutter speed? What information does this provide?
Since the tv96 value can be negative this line can be outside the histogram area and won't get erased leaving rubbish on the screen.

Quote

&& ptp & LV not quite congruent... (compression)
Code: [Select]
/platform/generic/wrappers.c ~~1638 // Y multiplier for cameras with 480 pixel high viewports (CHDK code assumes 240) int __attribute__((weak)) vid_get_viewport_yscale() { return 1; // For most cameras viewport is 240 pixels high }Digic6 defaults to 1, maybe some code expects 480 & return 2; together ?

Instead of guessing try spending some time understanding what the code does.

Quote

Edit2: Compiled same 6055 with
Quote
#ifndef THUMB_FW
return 1; // For most cameras viewport is 240 pixels high
#else
return 2;
#endif
, now the draw_vline() is not too long any more ....

Wrong - just wrong. Try using the edge overlay on a Digic6 camera with this change.

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)