After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 632506 - St: Fix blur radius computation to correspond to current CSS draft
St: Fix blur radius computation to correspond to current CSS draft
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Florian Müllner
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-10-18 21:32 UTC by Owen Taylor
Modified: 2011-03-22 19:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
St: Fix blur radius computation to correspond to current CSS draft (1.51 KB, patch)
2010-10-18 21:32 UTC, Owen Taylor
committed Details | Review

Description Owen Taylor 2010-10-18 21:32:54 UTC
The next draft of the CSS Backgrounds and Borders module will actually
define when the blur radius means. Fix our code to use that definition
(2 * standard deviation) rather than using the 1.9 * that we extracted
from what Mozilla was doing.
Comment 1 Owen Taylor 2010-10-18 21:32:56 UTC
Created attachment 172652 [details] [review]
St: Fix blur radius computation to correspond to current CSS draft
Comment 2 Owen Taylor 2010-10-18 21:38:32 UTC
Came up with this when I was looking at the code again to figure out how this would relate to the specified shadow radius for window shadows; the 1.9 we were using rang a bell as the '3 * sqrt(2*pi))' factor that separates a box blur approximation from a Gaussian filter that I had just used it Mutter, and so I looked around further to see if there was something that made sense.

(The Mozilla code in this area has changed recently too - see https://bugzilla.mozilla.org/show_bug.cgi?id=590039, though I'm not completely sure exactly what changed)
Comment 3 Florian Müllner 2010-10-19 16:22:49 UTC
Review of attachment 172652 [details] [review]:

You may want to reconsider the URL in the comment before committing.

::: src/st/st-private.c
@@ +379,3 @@
 
+  /* The CSS specification defines (or will define) the blur radius as twice
+   * the Gaussian standard deviation. See:

Hmm, I wish it would - this is the "specification" I found:

"The exact algorithm is not defined; however the resulting shadow must approximate (with each pixel being within 5% of its expected value) the image that would be generated by applying to the shadow a Gaussian blur with a standard deviation equal to half the blur radius"

Using radius == 2 * sigma is obviously better than an approximation, I'm just wondering why it hasn't been specified like that in the first place ...

@@ +381,3 @@
+   * the Gaussian standard deviation. See:
+   *
+   * http://lists.w3.org/Archives/Public/www-style/2010Sep/0002.html

Is http://dev.w3.org/csswg/css3-background/#the-box-shadow a better reference?
Comment 4 Owen Taylor 2010-10-19 17:13:56 UTC
(In reply to comment #3)
> Review of attachment 172652 [details] [review]:
> 
> You may want to reconsider the URL in the comment before committing.
> 
> ::: src/st/st-private.c
> @@ +379,3 @@
> 
> +  /* The CSS specification defines (or will define) the blur radius as twice
> +   * the Gaussian standard deviation. See:
> 
> Hmm, I wish it would - this is the "specification" I found:
> 
> "The exact algorithm is not defined; however the resulting shadow must
> approximate (with each pixel being within 5% of its expected value) the image
> that would be generated by applying to the shadow a Gaussian blur with a
> standard deviation equal to half the blur radius"
> 
> Using radius == 2 * sigma is obviously better than an approximation, I'm just
> wondering why it hasn't been specified like that in the first place ...

I think the meaning of the spec is to define it like that then allow approximations that are only close, like the repeated box blur approximation used in my Mutter patch and in Mozilla.

> @@ +381,3 @@
> +   * the Gaussian standard deviation. See:
> +   *
> +   * http://lists.w3.org/Archives/Public/www-style/2010Sep/0002.html
> 
> Is http://dev.w3.org/csswg/css3-background/#the-box-shadow a better reference?

My assumption was that I shouldn't link to the "editors draft" URL, but that the archived discussion would be permanent. Eventually, presumably the main w3.org version will be updated and we could link to that, but I was hesitant to put a link in a comment to something that wasn't updated yet.
Comment 5 Florian Müllner 2010-10-19 17:52:09 UTC
(In reply to comment #4)
> > @@ +381,3 @@
> > +   * the Gaussian standard deviation. See:
> > +   *
> > +   * http://lists.w3.org/Archives/Public/www-style/2010Sep/0002.html
> > 
> > Is http://dev.w3.org/csswg/css3-background/#the-box-shadow a better reference?
> 
> My assumption was that I shouldn't link to the "editors draft" URL, but that
> the archived discussion would be permanent. Eventually, presumably the main
> w3.org version will be updated and we could link to that, but I was hesitant to
> put a link in a comment to something that wasn't updated yet.

Fair point.
Comment 6 Florian Müllner 2011-03-22 04:24:58 UTC
Ping?
Comment 7 Owen Taylor 2011-03-22 19:55:58 UTC
Merged up and pushed. We'll hope that making CSS shadows 5% smaller at the last
moment won't be noticeable :-)

Attachment 172652 [details] pushed as 281b2e9 - St: Fix blur radius computation to correspond to current CSS draft