[Rt-commit] rt branch, 4.2/subsample-logo, created. rt-4.2.4rc1-15-g15c49d7
Alex Vandiver
alexmv at bestpractical.com
Mon May 12 15:03:14 EDT 2014
The branch, 4.2/subsample-logo has been created
at 15c49d7a97a330b52d5de2288ee8caf99192d001 (commit)
- Log -----------------------------------------------------------------
commit a1d56257a98e40ac3a30802eb853db450cd3c188
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon May 12 13:56:27 2014 -0400
Don't sample every pixel of large logos
The analyze_img code examined every pixel of the image; for large
images, this caused the page to time out, thus making it impossible to
change the image away from the problematically-large one.
Subsample the image if it is more than 200px on a side, by sampling in
an even 200x200 grid. 200px was chosen as an arbitrary upper limit; 40k
points is still a large number to sample, but in testing can be done in
less than 10s.
Fixes #29929.
diff --git a/share/html/Admin/Tools/Theme.html b/share/html/Admin/Tools/Theme.html
index c569cd1..ed60100 100644
--- a/share/html/Admin/Tools/Theme.html
+++ b/share/html/Admin/Tools/Theme.html
@@ -327,8 +327,20 @@ sub analyze_img {
my $img = shift;
my $color;
- for my $i (0..$img->width-1) {
- for my $j (0..$img->height-1) {
+ my @wsamples;
+ my @hsamples;
+ if ($img->width > 200) {
+ @wsamples = map { int($img->width*($_/200)) } (0..199);
+ } else {
+ @wsamples = ( 0 .. $img->width - 1 );
+ }
+ if ($img->height > 200) {
+ @hsamples = map { int($img->height*($_/200)) } (0..199);
+ } else {
+ @hsamples = ( 0 .. $img->height - 1 );
+ }
+ for my $i (@wsamples) {
+ for my $j (@hsamples) {
my @color = $img->rgb( $img->getPixel($i,$j) );
my $hsl = Convert::Color->new('rgb:'.join(',',map { $_ / 255 } @color))->convert_to('hsl');
my $c = join(',', at color);
commit 3105457cf13ca424906e030970c3080bdbe22350
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon May 12 14:26:21 2014 -0400
gd_can need not be a global
diff --git a/share/html/Admin/Tools/Theme.html b/share/html/Admin/Tools/Theme.html
index ed60100..f76db81 100644
--- a/share/html/Admin/Tools/Theme.html
+++ b/share/html/Admin/Tools/Theme.html
@@ -61,7 +61,7 @@
<label for="logo-upload"><&|/l&>Upload a new logo</&>:</label>
<input type="file" name="logo-upload" id="logo-upload" /><br />
<div class="gd-support">
-% if (%gd_can) {
+% if ($valid_image_types) {
<&|/l, $valid_image_types &>Your system supports automatic color suggestions for: [_1]</&>
% } else {
<&|/l&>GD is disabled or not installed. You can upload an image, but you won't get automatic color suggestions.</&>
@@ -298,13 +298,13 @@ use List::MoreUtils qw(uniq);
my $has_color_analyzer = eval { require Convert::Color; 1 };
my $colors;
-my %gd_can;
my $valid_image_types;
if (not RT->Config->Get('DisableGD') and $has_color_analyzer) {
require GD;
# Always find out what GD can read...
+ my %gd_can;
for my $type (qw(Png Jpeg Gif)) {
$gd_can{$type}++ if GD::Image->can("newFrom${type}Data");
}
commit 921c0011280a1ad8242728d68a0bdba8356c4e74
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon May 12 14:30:26 2014 -0400
Do not install a subroutine into the HTML::Mason::Commands namespace
sub {} in a Mason component makes the sub accessible to every Mason
component, and is highly discouraged. Use a local subref instead.
diff --git a/share/html/Admin/Tools/Theme.html b/share/html/Admin/Tools/Theme.html
index f76db81..79326f7 100644
--- a/share/html/Admin/Tools/Theme.html
+++ b/share/html/Admin/Tools/Theme.html
@@ -313,7 +313,7 @@ if (not RT->Config->Get('DisableGD') and $has_color_analyzer) {
# ...but only analyze the image if we have data
if ($imgdata) {
if ( my $img = GD::Image->new($imgdata) ) {
- $colors = analyze_img($img);
+ $colors = $analyze_img->($img);
}
else {
# This has to be one damn long line because the loc() needs to be
@@ -323,7 +323,7 @@ if (not RT->Config->Get('DisableGD') and $has_color_analyzer) {
}
}
-sub analyze_img {
+my $analyze_img = sub {
my $img = shift;
my $color;
@@ -359,7 +359,7 @@ sub analyze_img {
warn "bad";
}
return \@top5;
-}
+};
</%INIT>
<%ARGS>
$user_css => ''
commit a452b92566bd35871d1ed6a604a6dea37a9b258e
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon May 12 14:34:47 2014 -0400
Replace a needless hashref with a plain hash
diff --git a/share/html/Admin/Tools/Theme.html b/share/html/Admin/Tools/Theme.html
index 79326f7..431525d 100644
--- a/share/html/Admin/Tools/Theme.html
+++ b/share/html/Admin/Tools/Theme.html
@@ -325,7 +325,7 @@ if (not RT->Config->Get('DisableGD') and $has_color_analyzer) {
my $analyze_img = sub {
my $img = shift;
- my $color;
+ my %colors;
my @wsamples;
my @hsamples;
@@ -345,16 +345,16 @@ my $analyze_img = sub {
my $hsl = Convert::Color->new('rgb:'.join(',',map { $_ / 255 } @color))->convert_to('hsl');
my $c = join(',', at color);
next if $hsl->lightness < 0.1;
- $color->{$c} ||= { h => $hsl->hue, s => $hsl->saturation, l => $hsl->lightness, cnt => 0, c => $c};
- $color->{$c}->{cnt}++;
+ $colors{$c} ||= { h => $hsl->hue, s => $hsl->saturation, l => $hsl->lightness, cnt => 0, c => $c};
+ $colors{$c}->{cnt}++;
}
}
- for (values %$color) {
+ for (values %colors) {
$_->{rank} = $_->{s} * $_->{cnt};
}
my @top5 = grep { defined and $_->{'l'} and $_->{'c'} }
- (sort { $b->{rank} <=> $a->{rank} } values %$color)[0..5];
+ (sort { $b->{rank} <=> $a->{rank} } values %colors)[0..5];
if ((scalar uniq map {$_->{rank}} @top5) == 1) {
warn "bad";
}
commit 4b030a81b32a08b0c851ee11cc2bce9dc99e84e1
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon May 12 14:36:15 2014 -0400
Move List::MoreUtils closer to where it is used
diff --git a/share/html/Admin/Tools/Theme.html b/share/html/Admin/Tools/Theme.html
index 431525d..a12006a 100644
--- a/share/html/Admin/Tools/Theme.html
+++ b/share/html/Admin/Tools/Theme.html
@@ -294,8 +294,6 @@ if (!$user_css) {
# XXX: move this to some other modules
-use List::MoreUtils qw(uniq);
-
my $has_color_analyzer = eval { require Convert::Color; 1 };
my $colors;
my $valid_image_types;
@@ -350,6 +348,7 @@ my $analyze_img = sub {
}
}
+ use List::MoreUtils qw(uniq);
for (values %colors) {
$_->{rank} = $_->{s} * $_->{cnt};
}
commit 046fa97a384efdfd8c4a211079a6267df6583170
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon May 12 14:36:50 2014 -0400
Remove an unnecessary warning triggered by single-colored logos
diff --git a/share/html/Admin/Tools/Theme.html b/share/html/Admin/Tools/Theme.html
index a12006a..aa61a79 100644
--- a/share/html/Admin/Tools/Theme.html
+++ b/share/html/Admin/Tools/Theme.html
@@ -354,9 +354,6 @@ my $analyze_img = sub {
}
my @top5 = grep { defined and $_->{'l'} and $_->{'c'} }
(sort { $b->{rank} <=> $a->{rank} } values %colors)[0..5];
- if ((scalar uniq map {$_->{rank}} @top5) == 1) {
- warn "bad";
- }
return \@top5;
};
</%INIT>
commit e6b3c403432479f058b402454c96abbbc7da37e5
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon May 12 14:43:10 2014 -0400
Move GD checking into $analyze_img
diff --git a/share/html/Admin/Tools/Theme.html b/share/html/Admin/Tools/Theme.html
index aa61a79..6b4bb65 100644
--- a/share/html/Admin/Tools/Theme.html
+++ b/share/html/Admin/Tools/Theme.html
@@ -293,12 +293,12 @@ if (!$user_css) {
}
# XXX: move this to some other modules
-
-my $has_color_analyzer = eval { require Convert::Color; 1 };
my $colors;
my $valid_image_types;
+my $analyze_img = sub {
+ return undef if RT->Config->Get('DisableGD');
+ return undef unless eval { require Convert::Color; 1 };
-if (not RT->Config->Get('DisableGD') and $has_color_analyzer) {
require GD;
# Always find out what GD can read...
@@ -308,21 +308,18 @@ if (not RT->Config->Get('DisableGD') and $has_color_analyzer) {
}
$valid_image_types = join(", ", map { uc } sort { lc $a cmp lc $b } keys %gd_can);
+ my $imgdata = shift;
+ return undef unless $imgdata;
+
# ...but only analyze the image if we have data
- if ($imgdata) {
- if ( my $img = GD::Image->new($imgdata) ) {
- $colors = $analyze_img->($img);
- }
- else {
- # This has to be one damn long line because the loc() needs to be
- # source parsed correctly.
- push @results, loc("Automatically suggested theme colors aren't available for your image. This might be because you uploaded an image type that your installed version of GD doesn't support. Supported types are: [_1]. You can recompile libgd and GD.pm to include support for other image types.", $valid_image_types);
- }
+ my $img = GD::Image->new($imgdata);
+ unless ($img) {
+ # This has to be one damn long line because the loc() needs to be
+ # source parsed correctly.
+ push @results, loc("Automatically suggested theme colors aren't available for your image. This might be because you uploaded an image type that your installed version of GD doesn't support. Supported types are: [_1]. You can recompile libgd and GD.pm to include support for other image types.", $valid_image_types);
+ return undef;
}
-}
-my $analyze_img = sub {
- my $img = shift;
my %colors;
my @wsamples;
@@ -356,6 +353,7 @@ my $analyze_img = sub {
(sort { $b->{rank} <=> $a->{rank} } values %colors)[0..5];
return \@top5;
};
+$analyze_img->($imgdata);
</%INIT>
<%ARGS>
$user_css => ''
commit 3a84e3d5f12a6011bb229787779e67ff85c7546c
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon May 12 14:47:22 2014 -0400
Move $analyze_img earlier
diff --git a/share/html/Admin/Tools/Theme.html b/share/html/Admin/Tools/Theme.html
index 6b4bb65..49e1eb7 100644
--- a/share/html/Admin/Tools/Theme.html
+++ b/share/html/Admin/Tools/Theme.html
@@ -242,57 +242,6 @@ my $text_threshold = 0.6;
my @results;
my $imgdata;
-if (my $file_hash = _UploadedFile( 'logo-upload' )) {
- my ($id, $msg) = RT->System->SetAttribute( Name => "UserLogo",
- Description => "User-provided logo",
- Content => {
- type => $file_hash->{ContentType},
- data => $file_hash->{LargeContent},
- hash => md5_hex($file_hash->{LargeContent}),
- } );
- push @results, loc("Unable to set UserLogo: [_1]", $msg) unless $id;
-
- $imgdata = $file_hash->{LargeContent};
-}
-elsif ($ARGS{'reset_logo'}) {
- RT->System->DeleteAttribute('UserLogo');
-}
-else {
- if (my $attr = RT->System->FirstAttribute('UserLogo')) {
- my $content = $attr->Content;
- if (ref($content) eq 'HASH') {
- $imgdata = $content->{data};
- }
- else {
- RT->System->DeleteAttribute('UserLogo');
- }
- }
-}
-
-if ($user_css) {
- if ($ARGS{'reset_css'}) {
- RT->System->DeleteAttribute('UserCSS');
- undef $user_css;
- }
- else {
- my ($id, $msg) = RT->System->SetAttribute( Name => "UserCSS",
- Description => "User-provided css",
- Content => $user_css );
- push @results, loc("Unable to set UserCSS: [_1]", $msg) unless $id;
- }
-}
-
-if (!$user_css) {
- my $attr = RT->System->FirstAttribute('UserCSS');
- $user_css = $attr ? $attr->Content : join(
- "\n\n" => map {
- join "\n" => "/* ". $_->[0] ." */",
- map { "$_ {}" } @{$_->[1]}
- } @sections
- );
-}
-
-# XXX: move this to some other modules
my $colors;
my $valid_image_types;
my $analyze_img = sub {
@@ -353,7 +302,57 @@ my $analyze_img = sub {
(sort { $b->{rank} <=> $a->{rank} } values %colors)[0..5];
return \@top5;
};
+
+if (my $file_hash = _UploadedFile( 'logo-upload' )) {
+ my ($id, $msg) = RT->System->SetAttribute( Name => "UserLogo",
+ Description => "User-provided logo",
+ Content => {
+ type => $file_hash->{ContentType},
+ data => $file_hash->{LargeContent},
+ hash => md5_hex($file_hash->{LargeContent}),
+ } );
+ push @results, loc("Unable to set UserLogo: [_1]", $msg) unless $id;
+
+ $imgdata = $file_hash->{LargeContent};
+}
+elsif ($ARGS{'reset_logo'}) {
+ RT->System->DeleteAttribute('UserLogo');
+}
+else {
+ if (my $attr = RT->System->FirstAttribute('UserLogo')) {
+ my $content = $attr->Content;
+ if (ref($content) eq 'HASH') {
+ $imgdata = $content->{data};
+ }
+ else {
+ RT->System->DeleteAttribute('UserLogo');
+ }
+ }
+}
$analyze_img->($imgdata);
+
+if ($user_css) {
+ if ($ARGS{'reset_css'}) {
+ RT->System->DeleteAttribute('UserCSS');
+ undef $user_css;
+ }
+ else {
+ my ($id, $msg) = RT->System->SetAttribute( Name => "UserCSS",
+ Description => "User-provided css",
+ Content => $user_css );
+ push @results, loc("Unable to set UserCSS: [_1]", $msg) unless $id;
+ }
+}
+
+if (!$user_css) {
+ my $attr = RT->System->FirstAttribute('UserCSS');
+ $user_css = $attr ? $attr->Content : join(
+ "\n\n" => map {
+ join "\n" => "/* ". $_->[0] ." */",
+ map { "$_ {}" } @{$_->[1]}
+ } @sections
+ );
+}
</%INIT>
<%ARGS>
$user_css => ''
commit 15c49d7a97a330b52d5de2288ee8caf99192d001
Author: Alex Vandiver <alexmv at bestpractical.com>
Date: Mon May 12 14:47:52 2014 -0400
Cache the found colors in the logo attribute
As determining these colors may be time-intensive, and is guaranteed to
not change while the logo itself does not change, the information should
simply be stored alongside the logo.
diff --git a/share/html/Admin/Tools/Theme.html b/share/html/Admin/Tools/Theme.html
index 49e1eb7..41a9f84 100644
--- a/share/html/Admin/Tools/Theme.html
+++ b/share/html/Admin/Tools/Theme.html
@@ -304,13 +304,17 @@ my $analyze_img = sub {
};
if (my $file_hash = _UploadedFile( 'logo-upload' )) {
- my ($id, $msg) = RT->System->SetAttribute( Name => "UserLogo",
- Description => "User-provided logo",
- Content => {
- type => $file_hash->{ContentType},
- data => $file_hash->{LargeContent},
- hash => md5_hex($file_hash->{LargeContent}),
- } );
+ $colors = $analyze_img->($file_hash->{LargeContent});
+ my ($id, $msg) = RT->System->SetAttribute(
+ Name => "UserLogo",
+ Description => "User-provided logo",
+ Content => {
+ type => $file_hash->{ContentType},
+ data => $file_hash->{LargeContent},
+ hash => md5_hex($file_hash->{LargeContent}),
+ colors => $colors,
+ }
+ );
push @results, loc("Unable to set UserLogo: [_1]", $msg) unless $id;
$imgdata = $file_hash->{LargeContent};
@@ -323,13 +327,25 @@ else {
my $content = $attr->Content;
if (ref($content) eq 'HASH') {
$imgdata = $content->{data};
+ $colors = $content->{colors};
+ unless ($colors) {
+ # No colors cached; attempt to generate them
+ $colors = $content->{colors} = $analyze_img->($content->{data});
+ if ($content->{colors}) {
+ # Found colors; update the attribute
+ RT->System->SetAttribute(
+ Name => "UserLogo",
+ Description => "User-provided logo",
+ Content => $content,
+ );
+ }
+ }
}
else {
RT->System->DeleteAttribute('UserLogo');
}
}
}
-$analyze_img->($imgdata);
if ($user_css) {
if ($ARGS{'reset_css'}) {
-----------------------------------------------------------------------
More information about the rt-commit
mailing list