Tweak to histogram.c: A proposal - General Discussion and Assistance - CHDK Forum  

Tweak to histogram.c: A proposal

  • 11 Replies
  • 1828 Views
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


Re: Tweak to histogram.c: A proposal
« Reply #1 on: 31 / January / 2020, 12:31:48 »
 :-[

Oops
X and Y !!!!!


*

Offline reyalp

  • ******
  • 13619
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 »
Don't forget what the H stands for.

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


*

Offline Caefix

  • *****
  • 775
  • 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 ...  :haha
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);
            }
   
All lifetime is a loan from eternity.

*

Offline philmoz

  • *****
  • 3376
    • Photos
Re: Tweak to histogram.c: A proposal
« Reply #5 on: 15 / January / 2022, 16:49:33 »
One if more to avoid some flicker ...  :haha
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.

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)

*

Offline Caefix

  • *****
  • 775
  • Sorry, busy deleting test shots...
Re: Tweak to histogram.c: A proposal
« Reply #6 on: 17 / January / 2022, 11:13:18 »
One if more to avoid some flicker ...  :haha

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;
            }
        }
All lifetime is a loan from eternity.

*

Offline Caefix

  • *****
  • 775
  • 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 ...  :xmas
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 ....
« Last Edit: 25 / January / 2022, 15:34:33 by Caefix »
All lifetime is a loan from eternity.


*

Offline philmoz

  • *****
  • 3376
    • Photos
Re: Tweak to histogram.c: A proposal
« Reply #8 on: 25 / January / 2022, 17:23:02 »
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 ...  :xmas
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.
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)

*

Offline Caefix

  • *****
  • 775
  • Sorry, busy deleting test shots...
Re: Tweak to histogram.c: A proposal
« Reply #9 on: 30 / January / 2022, 12:04:47 »
Try using the edge overlay on a Digic6 camera with this change.

Done.  :-[ :-X :o
All lifetime is a loan from eternity.

 

Related Topics