graphs.pl: Make canvas fixed size and limit cpufreq color index
Commit Message
This fixes an issue where systems with many CPU cores
run out of usable graph colors and canvas area.
It also unifies the canvas size for all graphs.
Discussion: https://community.ipfire.org/t/cpu-freq-graph-not-working-with-i5-1235u/9396
Fixes: #12890
Signed-off-by: Leo-Andres Hofmann <hofmann@leo-andres.de>
---
config/cfgroot/graphs.pl | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
Comments
Hello,
Reviewed-by: Michael Tremer <michael.tremer@ipfire.org>
> On 13 Mar 2023, at 07:35, Leo-Andres Hofmann <hofmann@leo-andres.de> wrote:
>
> This fixes an issue where systems with many CPU cores
> run out of usable graph colors and canvas area.
> It also unifies the canvas size for all graphs.
>
> Discussion: https://community.ipfire.org/t/cpu-freq-graph-not-working-with-i5-1235u/9396
>
> Fixes: #12890
>
> Signed-off-by: Leo-Andres Hofmann <hofmann@leo-andres.de>
> ---
> config/cfgroot/graphs.pl | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/config/cfgroot/graphs.pl b/config/cfgroot/graphs.pl
> index 3368e5aad..cefe0b90d 100644
> --- a/config/cfgroot/graphs.pl
> +++ b/config/cfgroot/graphs.pl
> @@ -30,8 +30,11 @@ require '/var/ipfire/general-functions.pl';
> require "${General::swroot}/lang.pl";
> require "${General::swroot}/header.pl";
>
> -# Graph image size in pixel
> -our %image_size = ('width' => 910, 'height' => 300);
> +# Approximate size of the final graph image including canvas and labeling (in pixels, mainly used for placeholders)
> +our %image_size = ('width' => 900, 'height' => 300);
> +
> +# Size of the actual data area within the image, without labeling (in pixels)
> +our %canvas_size = ('width' => 800, 'height' => 190);
>
> # List of all available time ranges
> our @time_ranges = ("hour", "day", "week", "month", "year");
> @@ -48,15 +51,12 @@ my @GRAPH_ARGS = (
> # For a more 'organic' look
> "--slope-mode",
>
> - # HxW define the size of the output image
> - "--full-size-mode",
> -
> # Watermark
> "-W www.ipfire.org",
>
> - # Default size
> - "-w $image_size{'width'}",
> - "-h $image_size{'height'}",
> + # Canvas width/height
> + "-w $canvas_size{'width'}",
> + "-h $canvas_size{'height'}",
>
> # Use alternative grid
> "--alt-y-grid",
> @@ -1090,18 +1090,19 @@ sub updatecpufreqgraph {
> "--color=SHADEA".$color{"color19"},
> "--color=SHADEB".$color{"color19"},
> "--color=BACK".$color{"color21"},
> - "COMMENT:".sprintf("%-10s",$Lang::tr{'caption'}),
> + "COMMENT:".sprintf("%-15s",$Lang::tr{'caption'}),
> "COMMENT:".sprintf("%15s",$Lang::tr{'maximal'}),
> "COMMENT:".sprintf("%15s",$Lang::tr{'average'}),
> "COMMENT:".sprintf("%15s",$Lang::tr{'minimal'}),
> "COMMENT:".sprintf("%15s",$Lang::tr{'current'})."\\j"
> );
>
> + my $j = 11;
> for(my $i = 0; $i < $cpucount; $i++) {
> - my $j=$i+1;
> + $j++; $j = 1 if $j > 20;
I think you could have written this slightly shorter by taking the modulus of j like so: $j = $i % 20;
But since this is not changing the behaviour, I am happy to see this merged.
-Michael
> push(@command,"DEF:cpu".$i."_=".$mainsettings{'RRDLOG'}."/collectd/localhost/cpufreq/cpufreq-".$i.".rrd:value:AVERAGE"
> ,"CDEF:cpu".$i."=cpu".$i."_,1000000,/"
> - ,"LINE1:cpu".$i.$color{"color1$j"}."A0:cpu ".$i." "
> + ,"LINE1:cpu".$i.$color{"color$j"}."A0:cpu ".$i." "
> ,"GPRINT:cpu".$i.":MAX:%3.0lf Mhz"
> ,"GPRINT:cpu".$i.":AVERAGE:%3.0lf Mhz"
> ,"GPRINT:cpu".$i.":MIN:%3.0lf Mhz"
> --
> 2.37.1.windows.1
>
Hi,
Am 13.03.2023 um 10:23 schrieb Michael Tremer:
> Hello,
>
> Reviewed-by: Michael Tremer <michael.tremer@ipfire.org>
>
>> On 13 Mar 2023, at 07:35, Leo-Andres Hofmann <hofmann@leo-andres.de> wrote:
>>
>> This fixes an issue where systems with many CPU cores
>> run out of usable graph colors and canvas area.
>> It also unifies the canvas size for all graphs.
>>
>> Discussion: https://community.ipfire.org/t/cpu-freq-graph-not-working-with-i5-1235u/9396
>>
>> Fixes: #12890
>>
>> Signed-off-by: Leo-Andres Hofmann <hofmann@leo-andres.de>
>> ---
>> config/cfgroot/graphs.pl | 23 ++++++++++++-----------
>> 1 file changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/config/cfgroot/graphs.pl b/config/cfgroot/graphs.pl
>> index 3368e5aad..cefe0b90d 100644
>> --- a/config/cfgroot/graphs.pl
>> +++ b/config/cfgroot/graphs.pl
>> @@ -30,8 +30,11 @@ require '/var/ipfire/general-functions.pl';
>> require "${General::swroot}/lang.pl";
>> require "${General::swroot}/header.pl";
>>
>> -# Graph image size in pixel
>> -our %image_size = ('width' => 910, 'height' => 300);
>> +# Approximate size of the final graph image including canvas and labeling (in pixels, mainly used for placeholders)
>> +our %image_size = ('width' => 900, 'height' => 300);
>> +
>> +# Size of the actual data area within the image, without labeling (in pixels)
>> +our %canvas_size = ('width' => 800, 'height' => 190);
>>
>> # List of all available time ranges
>> our @time_ranges = ("hour", "day", "week", "month", "year");
>> @@ -48,15 +51,12 @@ my @GRAPH_ARGS = (
>> # For a more 'organic' look
>> "--slope-mode",
>>
>> - # HxW define the size of the output image
>> - "--full-size-mode",
>> -
>> # Watermark
>> "-W www.ipfire.org",
>>
>> - # Default size
>> - "-w $image_size{'width'}",
>> - "-h $image_size{'height'}",
>> + # Canvas width/height
>> + "-w $canvas_size{'width'}",
>> + "-h $canvas_size{'height'}",
>>
>> # Use alternative grid
>> "--alt-y-grid",
>> @@ -1090,18 +1090,19 @@ sub updatecpufreqgraph {
>> "--color=SHADEA".$color{"color19"},
>> "--color=SHADEB".$color{"color19"},
>> "--color=BACK".$color{"color21"},
>> - "COMMENT:".sprintf("%-10s",$Lang::tr{'caption'}),
>> + "COMMENT:".sprintf("%-15s",$Lang::tr{'caption'}),
>> "COMMENT:".sprintf("%15s",$Lang::tr{'maximal'}),
>> "COMMENT:".sprintf("%15s",$Lang::tr{'average'}),
>> "COMMENT:".sprintf("%15s",$Lang::tr{'minimal'}),
>> "COMMENT:".sprintf("%15s",$Lang::tr{'current'})."\\j"
>> );
>>
>> + my $j = 11;
>> for(my $i = 0; $i < $cpucount; $i++) {
>> - my $j=$i+1;
>> + $j++; $j = 1 if $j > 20;
> I think you could have written this slightly shorter by taking the modulus of j like so: $j = $i % 20;
>
> But since this is not changing the behaviour, I am happy to see this merged.
Thanks for the review. Yes of course, no idea why I didn't think of that...
Best,
Leo
>
> -Michael
>
>> push(@command,"DEF:cpu".$i."_=".$mainsettings{'RRDLOG'}."/collectd/localhost/cpufreq/cpufreq-".$i.".rrd:value:AVERAGE"
>> ,"CDEF:cpu".$i."=cpu".$i."_,1000000,/"
>> - ,"LINE1:cpu".$i.$color{"color1$j"}."A0:cpu ".$i." "
>> + ,"LINE1:cpu".$i.$color{"color$j"}."A0:cpu ".$i." "
>> ,"GPRINT:cpu".$i.":MAX:%3.0lf Mhz"
>> ,"GPRINT:cpu".$i.":AVERAGE:%3.0lf Mhz"
>> ,"GPRINT:cpu".$i.":MIN:%3.0lf Mhz"
>> --
>> 2.37.1.windows.1
>>
@@ -30,8 +30,11 @@ require '/var/ipfire/general-functions.pl';
require "${General::swroot}/lang.pl";
require "${General::swroot}/header.pl";
-# Graph image size in pixel
-our %image_size = ('width' => 910, 'height' => 300);
+# Approximate size of the final graph image including canvas and labeling (in pixels, mainly used for placeholders)
+our %image_size = ('width' => 900, 'height' => 300);
+
+# Size of the actual data area within the image, without labeling (in pixels)
+our %canvas_size = ('width' => 800, 'height' => 190);
# List of all available time ranges
our @time_ranges = ("hour", "day", "week", "month", "year");
@@ -48,15 +51,12 @@ my @GRAPH_ARGS = (
# For a more 'organic' look
"--slope-mode",
- # HxW define the size of the output image
- "--full-size-mode",
-
# Watermark
"-W www.ipfire.org",
- # Default size
- "-w $image_size{'width'}",
- "-h $image_size{'height'}",
+ # Canvas width/height
+ "-w $canvas_size{'width'}",
+ "-h $canvas_size{'height'}",
# Use alternative grid
"--alt-y-grid",
@@ -1090,18 +1090,19 @@ sub updatecpufreqgraph {
"--color=SHADEA".$color{"color19"},
"--color=SHADEB".$color{"color19"},
"--color=BACK".$color{"color21"},
- "COMMENT:".sprintf("%-10s",$Lang::tr{'caption'}),
+ "COMMENT:".sprintf("%-15s",$Lang::tr{'caption'}),
"COMMENT:".sprintf("%15s",$Lang::tr{'maximal'}),
"COMMENT:".sprintf("%15s",$Lang::tr{'average'}),
"COMMENT:".sprintf("%15s",$Lang::tr{'minimal'}),
"COMMENT:".sprintf("%15s",$Lang::tr{'current'})."\\j"
);
+ my $j = 11;
for(my $i = 0; $i < $cpucount; $i++) {
- my $j=$i+1;
+ $j++; $j = 1 if $j > 20;
push(@command,"DEF:cpu".$i."_=".$mainsettings{'RRDLOG'}."/collectd/localhost/cpufreq/cpufreq-".$i.".rrd:value:AVERAGE"
,"CDEF:cpu".$i."=cpu".$i."_,1000000,/"
- ,"LINE1:cpu".$i.$color{"color1$j"}."A0:cpu ".$i." "
+ ,"LINE1:cpu".$i.$color{"color$j"}."A0:cpu ".$i." "
,"GPRINT:cpu".$i.":MAX:%3.0lf Mhz"
,"GPRINT:cpu".$i.":AVERAGE:%3.0lf Mhz"
,"GPRINT:cpu".$i.":MIN:%3.0lf Mhz"