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 624383 - multipoint gradients
multipoint gradients
Status: RESOLVED OBSOLETE
Product: gnome-shell
Classification: Core
Component: st
3.15.x
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 587003 737789 787434 (view as bug list)
Depends on: 633460
Blocks:
 
 
Reported: 2010-07-14 20:15 UTC by Jakub Steiner
Modified: 2021-07-05 14:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to add some CSS3 features support (55.72 KB, patch)
2011-01-16 15:08 UTC, Quentin "Sardem FF7" Glidic
none Details | Review
Implementation of CSS3 gradients (33.81 KB, patch)
2011-01-16 17:29 UTC, Quentin "Sardem FF7" Glidic
none Details | Review
Implementation of CSS3 gradients (32.88 KB, patch)
2011-01-17 17:07 UTC, Quentin "Sardem FF7" Glidic
none Details | Review
Implementation of CSS3 gradients (33.48 KB, patch)
2011-01-17 20:29 UTC, Quentin "Sardem FF7" Glidic
none Details | Review
Implementation of CSS3 gradients (35.22 KB, patch)
2011-01-19 12:37 UTC, Quentin "Sardem FF7" Glidic
none Details | Review
Implementation of CSS3 gradients (background and background-image properties) (35.28 KB, patch)
2011-01-19 12:45 UTC, Quentin "Sardem FF7" Glidic
needs-work Details | Review
Implementation of CSS3 gradients (background and background-image properties) (37.85 KB, patch)
2011-07-10 21:59 UTC, Quentin "Sardem FF7" Glidic
none Details | Review

Description Jakub Steiner 2010-07-14 20:15:14 UTC
The current linear gradient CSS-like property only allows to define a start point and endpoint.

For things like entry widgets, it would be beneficial to be able to define multiple color nodes. Ideally this should use the syntax proposed for CSS3 rather than our own, allowing arbirary angle as well -- http://dev.w3.org/csswg/css3-images/#linear-gradients.

But I'm fine with having our own properties.
Comment 1 Florian Müllner 2010-07-16 06:41:55 UTC
*** Bug 587003 has been marked as a duplicate of this bug. ***
Comment 2 Quentin "Sardem FF7" Glidic 2011-01-16 15:08:08 UTC
Created attachment 178443 [details] [review]
Patch to add some CSS3 features support

This is a big patch with three features adding:
- linear/radial-gradient
- background-position
- background-size

Comments added to corresponding bugs.
Comment 3 Florian Müllner 2011-01-16 15:46:31 UTC
(In reply to comment #2)
> This is a big patch with three features adding:
> - linear/radial-gradient
> - background-position
> - background-size

... so it should be a minimum of three patches. Seriously, can you split this up?
Comment 4 Quentin "Sardem FF7" Glidic 2011-01-16 17:29:32 UTC
Created attachment 178450 [details] [review]
Implementation of CSS3 gradients

Splitted patch for CSS3 gradients feature

This patch needs background-position from bug 633460.
Comment 5 Quentin "Sardem FF7" Glidic 2011-01-17 17:07:57 UTC
Created attachment 178531 [details] [review]
Implementation of CSS3 gradients

Little corrections to the background clean part.
Comment 6 Quentin "Sardem FF7" Glidic 2011-01-17 20:29:13 UTC
Created attachment 178558 [details] [review]
Implementation of CSS3 gradients

Forgot StThemeNode -private.h change
Comment 7 Quentin "Sardem FF7" Glidic 2011-01-19 12:37:56 UTC
Created attachment 178716 [details] [review]
Implementation of CSS3 gradients

Rebase on top of the new background-size
Comment 8 Quentin "Sardem FF7" Glidic 2011-01-19 12:45:03 UTC
Created attachment 178718 [details] [review]
Implementation of CSS3 gradients (background and background-image properties)

Oops, forgot I add gradients to background-image (so didn't test to compile it).
Comment 9 Jasper St. Pierre (not reading bugmail) 2011-07-08 09:06:33 UTC
Review of attachment 178718 [details] [review]:

Good. Just watch your code style, we're using GNU for our C code. This means that blocks look like:

  if (foo)
    {
      bar = 1;
    }

and parenthesis need a space before them.

::: src/st/st-gradient.c
@@ +22,3 @@
+
+StGradient *
+st_gradient_new(StGradientType type)

Is there any reason we're not using a full GObject here?

@@ +116,3 @@
+
+void
+st_gradient_set_rx(StGradient *gradient, gdouble r)

Some documentation on what "rx" and "ry" mean would be useful.

@@ +139,3 @@
+    return;
+  
+  if (offset>100)

Might want a comment defining what "100" is, with a reference to a W3C primary source if possible.

::: src/st/st-theme-node-drawing.c
@@ +474,3 @@
 }
 
+#define DISTANCE(x0, y0, x1, y1) (sqrt((fabs(x1-x0)*fabs(x1-x0))+(fabs(y1-y0)*fabs(y1-y0))))

I don't think we really want a sqrt here. If we're trying to get the minimum distance, won't a Manhattan work just as well?

@@ +538,3 @@
+      gdouble h, v;
+      gdouble tl, tr, br, bl;
+      gdouble sx = 1, sy = 1;

Can you use more descriptive variable names?
Comment 10 Quentin "Sardem FF7" Glidic 2011-07-10 21:59:02 UTC
Created attachment 191653 [details] [review]
Implementation of CSS3 gradients (background and background-image properties)

Real patch here. That's the last time I promise. The patch (as the others) is just the last one I rebased, no review comment integrated yet, you may not repeat yourself so.

(In reply to comment #9)
> Review of attachment 178718 [details] [review]:
> 
> Good. Just watch your code style, we're using GNU for our C code. This means
> that blocks look like:
> 
>   if (foo)
>     {
>       bar = 1;
>     }
> 
> and parenthesis need a space before them.

I will try, that's definitely not my favorite style.


> ::: src/st/st-gradient.c
> @@ +22,3 @@
> +
> +StGradient *
> +st_gradient_new(StGradientType type)
> 
> Is there any reason we're not using a full GObject here?

Not sure, I think I copied the other ST struct way to do.


> @@ +116,3 @@
> +
> +void
> +st_gradient_set_rx(StGradient *gradient, gdouble r)
> 
> Some documentation on what "rx" and "ry" mean would be useful.
> 
> @@ +139,3 @@
> +    return;
> +  
> +  if (offset>100)
> 
> Might want a comment defining what "100" is, with a reference to a W3C primary
> source if possible.

Noted.


> ::: src/st/st-theme-node-drawing.c
> @@ +474,3 @@
>  }
> 
> +#define DISTANCE(x0, y0, x1, y1)
> (sqrt((fabs(x1-x0)*fabs(x1-x0))+(fabs(y1-y0)*fabs(y1-y0))))
> 
> I don't think we really want a sqrt here. If we're trying to get the minimum
> distance, won't a Manhattan work just as well?

I didn't know the Manhattan distance, so something that simple would work?
#define DISTANCE(x0, y0, x1, y1) (fabs(x1-x0)+fabs(y1-y0))

Interesting, just need some tests, I hope.


> @@ +538,3 @@
> +      gdouble h, v;
> +      gdouble tl, tr, br, bl;
> +      gdouble sx = 1, sy = 1;
> 
> Can you use more descriptive variable names?

Yes, we can.


I might delegate the tests right now, until I got some gnome-shell environment to test it myself (not at home).
Comment 11 Jasper St. Pierre (not reading bugmail) 2011-07-10 22:51:27 UTC
> I didn't know the Manhattan distance, so something that simple would work?
> #define DISTANCE(x0, y0, x1, y1) (fabs(x1-x0)+fabs(y1-y0))

A Manhattan difference has a different locus than a Euclidian one: the former is a diamond, the latter is a circle. I'm not sure it matters when finding the closest/farthest corner of a rectangle. But since we're doing that, why not check the quadrant we're in and let cairo do the math for us?

static void
closest_corner (int  x,
                int  y,
                int  width,
                int  height
                int *corner_x,
                int *corner_y)
{
  /* This function assumes that we have a rectangle from (0, 0)
   * to (width, height) and that (x, y) is in that rectangle. */

  if (corner_x)
    {
      if (x <= (width / 2))
        *corner_x = 0;
      else
        *corner_x = width;
    }

  if (corner_y)
    {
      if (y <= (height / 2))
        *corner_y = 0;
      else
        *corner_y = height;
    }
}

and

int corner_x, corner_y;
get_closest_corner (x, y, width, height, &corner_x, &corner_y);
pattern = cairo_pattern_create_radial (x, y, 0, corner_x, corner_y, 0);
Comment 12 Florian Müllner 2014-10-02 20:32:02 UTC
*** Bug 737789 has been marked as a duplicate of this bug. ***
Comment 13 Jakub Steiner 2014-10-06 10:54:38 UTC
The duplicate I f(a)iled tried to emphasize the benefit of having the same way to define gradients across Adwaita gtk and the shell.
Comment 14 Florian Müllner 2017-09-08 11:38:34 UTC
*** Bug 787434 has been marked as a duplicate of this bug. ***
Comment 15 GNOME Infrastructure Team 2021-07-05 14:05:02 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of  gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new ticket at
  https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/

Thank you for your understanding and your help.