Unity

Merge lp:~unity-team/unity/new-dash-blending into lp:unity

  1. new-dash-blending
  2. Merge into trunk
Proposed by Robert Carr
Status: Merged
Approved by: Robert Carr
Approved revision: no longer in the source branch.
Merged at revision: 2130
Proposed branch: lp:~unity-team/unity/new-dash-blending
Merge into: lp:unity
Diff against target: 404 lines (+191/-55)
5 files modified
plugins/unityshell/src/BGHash.cpp (+10/-7)
plugins/unityshell/src/Launcher.cpp (+29/-8)
plugins/unityshell/src/OverlayRenderer.cpp (+58/-16)
plugins/unityshell/src/PanelView.cpp (+59/-15)
plugins/unityshell/src/UnityWindowView.cpp (+35/-9)
To merge this branch: bzr merge lp:~unity-team/unity/new-dash-blending
Reviewer Review Type Date Requested Status
Robert Carr (community) Approve
Tim Penhey (community) Approve
Jay Taoko (community) Approve
Review via email: mp+94472@code.launchpad.net

Commit message

Implement new dash blending algorithm on cards with GLSL support.

Description of the change

Implement the new blending heuristic proposed in: https://bugs.launchpad.net/unity/+bug/865239

Thanks to Cimi for help with this, espescially with tweaking the colors.

Depends on: https://code.launchpad.net/~unity-team/nux/texture-blend/+merge/94596

Rosie and Cimi have signed off.

To post a comment you must log in.
Revision history for this message
Robert Carr (robertcarr) wrote :

http://i.imgur.com/PvlCO.png
http://i.imgur.com/yY2yS.png

Screenshots from Cimi

Revision history for this message
Tim Penhey (thumper) wrote :

On 24/02/12 12:55, Robert Carr wrote:
> http://i.imgur.com/PvlCO.png
> http://i.imgur.com/yY2yS.png
>
> Screenshots from Cimi

Is it just me, or is the only difference the background ?

I can't tell what has changed at all.

Revision history for this message
Andrea Cimitan (cimi) wrote :

> On 24/02/12 12:55, Robert Carr wrote:
> > http://i.imgur.com/PvlCO.png
> > http://i.imgur.com/yY2yS.png
> >
> > Screenshots from Cimi
>
> Is it just me, or is the only difference the background ?
>
> I can't tell what has changed at all.

Open the dash with the current unity, the background color of the dash is different

Revision history for this message
Robert Carr (robertcarr) wrote :

Here is a picture with no color profile that might indicate the blending changes more clearly:
http://i.imgur.com/bxu4p.png . It's really hard to talk about changes like this with our uncalibrated screens...for example Cimis pictures dont look anything like what I see on my screen ? Part of it has to do with browsers doing their own color management :p

I would say its roughly 9000% more purple.

The main point though, is the Overlay blend is more cooperative with heavily white backgrounds, as it blends differently based on the luminosity of the colorization. Overall trying to reduce the amount of white in the background so we have less problems with text.

Try it out :) It's very very noticeable when running it.

Revision history for this message
Robert Carr (robertcarr) wrote :

Also I'm asleep right now.

review: Abstain
Revision history for this message
Tim Penhey (thumper) wrote :

On Fri 24 Feb 2012 14:52:17 NZDT, Robert Carr wrote:
> Review: Abstain
>
> Also I'm asleep right now.

I can tell... your are typing slowly

Revision history for this message
Robert Carr (robertcarr) wrote :

Nux branch is now at: https://code.launchpad.net/~unity-team/nux/texture-blend/+merge/94596

Revision history for this message
Tim Penhey (thumper) wrote :

I'd like to see all arbitrary design constants extract out of the source into at least a const section in a source file, and use the named constants in the source.

I see the same paint code four times with an ifdef in it. Please abstract that into a function.

review: Needs Fixing
Revision history for this message
Robert Carr (robertcarr) wrote :

Yes sorry, waiting until the nux branch finishes (just redid the API again) to clean it all up...not really ready for review yet

Revision history for this message
Andrea Cimitan (cimi) wrote :

> Yes sorry, waiting until the nux branch finishes (just redid the API again) to
> clean it all up...not really ready for review yet

Robert, mark the reviews "Work in progress" if they are not ready yet, you save time to reviewers (especially if they reviewed in their spare time, like Tim :-))!

Revision history for this message
Jay Taoko (jaytaoko) wrote :

This work depends on this branch:

https://code.launchpad.net/~unity-team/nux/texture-blend/+merge/94596

The nux branch as been reviewed and approved. Test this branch again before merging.

Revision history for this message
Andrea Cimitan (cimi) wrote :

Will do monday morning ;-)

Revision history for this message
Jay Taoko (jaytaoko) :
review: Approve
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Are we sure the saturation is not a bit over the top? On top of a black terminal the dash/switcher bg is nearly black and over lighter backgrounds the color stands out so hard it looks a bit "unreal" or how to put it...

Revision history for this message
Andrea Cimitan (cimi) wrote :

Mikkel, we are aware of the different colorization on dark bg. Probably we might want to slightly refine the saturation, but the improvement of this branch is already greately appreciated!

Revision history for this message
Tim Penhey (thumper) wrote :

There are still #ifdefs here, if nux now supports the method please remove them.

Also, since the same drawing code is in a number of places, please extract into a function.

review: Needs Fixing
Revision history for this message
Robert Carr (robertcarr) wrote :

Hi Tim.

The IfDefs are intentional, and not related to Nux. The flow is so that on OpenGL ES2 when we always have GLSL we can avoid having to check at runtime if we are using the GLSL code path. The ifdefs could of course be removed, with the second case gone.

Not sure exactly how you want the drawing code extracted either. If you mean for example, lines 61-83 should be in a function because something similar is used elsewhere (247-271), I suppose that could be done. If we want to introduce three functions which are just, if (GLSL) Blend else DrawTexture using bla method, it should probably be in Nux instead. Jay?

The real problem is that the same window rendering code is replicated across various views, panel, etc...it's not purely a matter of copy paste to solve this at the moment though because of DrawContent, and some coupling with drawing and internal state (like in PanelView) so it doesn't seem like an appropriate time to try and fix that.

Revision history for this message
Tim Penhey (thumper) wrote :

/me sighs.

OK... I relent.

review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

No commit message specified.

Revision history for this message
Robert Carr (robertcarr) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/unityshell/src/BGHash.cpp'
2--- plugins/unityshell/src/BGHash.cpp 2012-02-22 13:40:18 +0000
3+++ plugins/unityshell/src/BGHash.cpp 2012-03-13 02:55:22 +0000
4@@ -395,10 +395,10 @@
5 ubus_server_send_message(ubus_server_get_default(),
6 UBUS_BACKGROUND_COLOR_CHANGED,
7 g_variant_new ("(dddd)",
8- _current_color.red * 0.7f,
9- _current_color.green * 0.7f,
10- _current_color.blue * 0.7f,
11- 0.5)
12+ _current_color.red,
13+ _current_color.green,
14+ _current_color.blue,
15+ _current_color.alpha)
16 );
17 }
18
19@@ -712,6 +712,7 @@
20 // grayscale image
21 LOG_DEBUG (logger) << "got a grayscale image";
22 chosen_color = nux::Color (46 , 52 , 54 );
23+ chosen_color.alpha = 0.72f;
24 }
25 else
26 {
27@@ -731,13 +732,15 @@
28
29 nux::color::HueSaturationValue hsv_color (chosen_color);
30
31- hsv_color.saturation = std::min(base_hsv.saturation, hsv_color.saturation) * 1.36f;
32- hsv_color.value = std::min(std::min(base_hsv.value, hsv_color.value), 0.2f);
33+ hsv_color.saturation = std::min(base_hsv.saturation, hsv_color.saturation) * 1.3f;
34+ hsv_color.value = std::min(std::min(base_hsv.value, hsv_color.value), 0.26f);
35 chosen_color = nux::Color (nux::color::RedGreenBlue(hsv_color));
36+
37+ // Reduce alpha on really dark average colors
38+ chosen_color.alpha = 0.72f - 2 * (0.26f - hsv_color.value);
39 }
40
41 // apply design to the colour
42- chosen_color.alpha = 0.5f;
43
44 LOG_DEBUG(logger) << "eventually chose "
45 << chosen_color.red << ", "
46
47=== modified file 'plugins/unityshell/src/Launcher.cpp'
48--- plugins/unityshell/src/Launcher.cpp 2012-02-29 16:09:07 +0000
49+++ plugins/unityshell/src/Launcher.cpp 2012-03-13 02:55:22 +0000
50@@ -1869,12 +1869,30 @@
51 texxform_blur_bg.voffset = ((float) base.y) / geo_absolute.height;
52
53 GfxContext.PushClippingRectangle(bkg_box);
54- gPainter.PushDrawTextureLayer(GfxContext, base,
55- blur_texture,
56- texxform_blur_bg,
57- nux::color::White,
58- true,
59- ROP);
60+
61+#ifndef NUX_OPENGLES_20
62+ if (GfxContext.UsingGLSLCodePath())
63+ gPainter.PushDrawCompositionLayer(GfxContext, base,
64+ blur_texture,
65+ texxform_blur_bg,
66+ nux::color::White,
67+ _background_color, nux::LAYER_BLEND_MODE_OVERLAY,
68+ true, ROP);
69+ else
70+ gPainter.PushDrawTextureLayer(GfxContext, base,
71+ blur_texture,
72+ texxform_blur_bg,
73+ nux::color::White,
74+ true,
75+ ROP);
76+#else
77+ gPainter.PushDrawCompositionLayer(GfxContext, base,
78+ blur_texture,
79+ texxform_blur_bg,
80+ nux::color::White,
81+ _background_color, nux::LAYER_BLEND_MODE_OVERLAY,
82+ true, ROP);
83+#endif
84 GfxContext.PopClippingRectangle();
85
86 push_count++;
87@@ -1886,11 +1904,14 @@
88
89 // apply the darkening
90 GfxContext.GetRenderStates().SetBlend(true, GL_ZERO, GL_SRC_COLOR);
91- gPainter.Paint2DQuadColor(GfxContext, bkg_box, nux::Color(0.7f, 0.7f, 0.7f, 1.0f));
92+ gPainter.Paint2DQuadColor(GfxContext, bkg_box, nux::Color(0.9f, 0.9f, 0.9f, 1.0f));
93 GfxContext.GetRenderStates().SetBlend (alpha, src, dest);
94
95 // apply the bg colour
96- gPainter.Paint2DQuadColor(GfxContext, bkg_box, _background_color);
97+#ifndef NUX_OPENGLES_20
98+ if (GfxContext.UsingGLSLCodePath() == FALSE)
99+ gPainter.Paint2DQuadColor(GfxContext, bkg_box, _background_color);
100+#endif
101
102 // apply the shine
103 GfxContext.GetRenderStates().SetBlend(true, GL_DST_COLOR, GL_ONE);
104
105=== modified file 'plugins/unityshell/src/OverlayRenderer.cpp'
106--- plugins/unityshell/src/OverlayRenderer.cpp 2012-02-07 07:42:12 +0000
107+++ plugins/unityshell/src/OverlayRenderer.cpp 2012-03-13 02:55:22 +0000
108@@ -97,7 +97,7 @@
109 rop.Blend = true;
110 rop.SrcBlend = GL_ZERO;
111 rop.DstBlend = GL_SRC_COLOR;
112- bg_darken_layer_ = new nux::ColorLayer(nux::Color(0.7f, 0.7f, 0.7f, 1.0f), false, rop);
113+ bg_darken_layer_ = new nux::ColorLayer(nux::Color(0.9f, 0.9f, 0.9f, 1.0f), false, rop);
114 bg_shine_texture_ = unity::dash::Style::Instance().GetDashShine()->GetDeviceTexture();
115
116 ubus_manager_.RegisterInterest(UBUS_BACKGROUND_COLOR_CHANGED,
117@@ -262,9 +262,24 @@
118 gfx_context.PushClippingRectangle(bg_clip);
119
120 gfx_context.GetRenderStates().SetBlend(false);
121- gfx_context.QRP_1Tex (content_geo.x, content_geo.y,
122- content_geo.width, content_geo.height,
123- bg_blur_texture_, texxform_absolute_bg, nux::color::White);
124+#ifndef NUX_OPENGLES_20
125+ if (gfx_context.UsingGLSLCodePath())
126+ gfx_context.QRP_GLSL_ColorBlendOverTex (content_geo.x, content_geo.y,
127+ content_geo.width, content_geo.height,
128+ bg_blur_texture_, texxform_absolute_bg, nux::color::White,
129+ bg_color_, nux::LAYER_BLEND_MODE_OVERLAY);
130+
131+ else
132+ gfx_context.QRP_1Tex (content_geo.x, content_geo.y,
133+ content_geo.width, content_geo.height,
134+ bg_blur_texture_, texxform_absolute_bg, nux::color::White);
135+#else
136+ gfx_context.QRP_GLSL_ColorBlendOverTex (content_geo.x, content_geo.y,
137+ content_geo.width, content_geo.height,
138+ bg_blur_texture_, texxform_absolute_bg, nux::color::White,
139+ bg_color_, nux::LAYER_BLEND_MODE_OVERLAY);
140+
141+#endif
142 gPainter.PopBackground();
143
144 gfx_context.PopClippingRectangle();
145@@ -301,9 +316,14 @@
146 // Draw the background
147 bg_darken_layer_->SetGeometry(content_geo);
148 nux::GetPainter().RenderSinglePaintLayer(gfx_context, content_geo, bg_darken_layer_);
149-
150- bg_layer_->SetGeometry(content_geo);
151- nux::GetPainter().RenderSinglePaintLayer(gfx_context, content_geo, bg_layer_);
152+
153+#ifndef NUX_OPENGLES_20
154+ if (gfx_context.UsingGLSLCodePath() == FALSE)
155+ {
156+ bg_layer_->SetGeometry(content_geo);
157+ nux::GetPainter().RenderSinglePaintLayer(gfx_context, content_geo, bg_layer_);
158+ }
159+#endif
160
161
162 texxform_absolute_bg.flip_v_coord = false;
163@@ -377,12 +397,29 @@
164
165 if (bg_blur_texture_.IsValid() && paint_blur)
166 {
167- gPainter.PushTextureLayer(gfx_context, content_geo,
168- bg_blur_texture_,
169- texxform_absolute_bg,
170- nux::color::White,
171- true, // write alpha?
172- rop);
173+#ifndef NUX_OPENGLES_20
174+ if (gfx_context.UsingGLSLCodePath())
175+ gPainter.PushCompositionLayer(gfx_context, content_geo,
176+ bg_blur_texture_,
177+ texxform_absolute_bg,
178+ nux::color::White,
179+ bg_color_, nux::LAYER_BLEND_MODE_OVERLAY,
180+ true, rop);
181+ else
182+ gPainter.PushTextureLayer(gfx_context, content_geo,
183+ bg_blur_texture_,
184+ texxform_absolute_bg,
185+ nux::color::White,
186+ true, // write alpha?
187+ rop);
188+#else
189+ gPainter.PushCompositionLayer(gfx_context, content_geo,
190+ bg_blur_texture_,
191+ texxform_absolute_bg,
192+ nux::color::White,
193+ bg_color_, nux::LAYER_BLEND_MODE_OVERLAY,
194+ true, rop);
195+#endif
196 bgs++;
197 }
198
199@@ -390,8 +427,13 @@
200 nux::GetPainter().PushLayer(gfx_context, bg_darken_layer_->GetGeometry(), bg_darken_layer_);
201 bgs++;
202
203- nux::GetPainter().PushLayer(gfx_context, bg_layer_->GetGeometry(), bg_layer_);
204- bgs++;
205+#ifndef NUX_OPENGLES_20
206+ if (gfx_context.UsingGLSLCodePath() == FALSE)
207+ {
208+ nux::GetPainter().PushLayer(gfx_context, bg_layer_->GetGeometry(), bg_layer_);
209+ bgs++;
210+ }
211+#endif
212
213 // apply the shine
214 rop.Blend = true;
215@@ -401,7 +443,7 @@
216 texxform_absolute_bg.uoffset = (1.0f / bg_shine_texture_->GetWidth()) * parent->x_offset;
217 texxform_absolute_bg.voffset = (1.0f / bg_shine_texture_->GetHeight()) * parent->y_offset;
218
219- nux::GetPainter().PushTextureLayer(gfx_context, bg_layer_->GetGeometry(),
220+ nux::GetPainter().PushTextureLayer(gfx_context, content_geo,
221 bg_shine_texture_,
222 texxform_absolute_bg,
223 nux::color::White,
224
225=== modified file 'plugins/unityshell/src/PanelView.cpp'
226--- plugins/unityshell/src/PanelView.cpp 2012-02-04 05:28:23 +0000
227+++ plugins/unityshell/src/PanelView.cpp 2012-03-13 02:55:22 +0000
228@@ -72,7 +72,7 @@
229 rop.Blend = true;
230 rop.SrcBlend = GL_ZERO;
231 rop.DstBlend = GL_SRC_COLOR;
232- _bg_darken_layer_ = new nux::ColorLayer(nux::Color(0.7f, 0.7f, 0.7f, 1.0f), false, rop);
233+ _bg_darken_layer_ = new nux::ColorLayer(nux::Color(0.9f, 0.9f, 0.9f, 1.0f), false, rop);
234
235 _layout = new nux::HLayout("", NUX_TRACKER_LOCATION);
236
237@@ -251,12 +251,31 @@
238 nux::Geometry bg_clip = geo;
239 GfxContext.PushClippingRectangle(bg_clip);
240
241- gPainter.PushDrawTextureLayer(GfxContext, geo,
242- bg_blur_texture_,
243- texxform_blur_bg,
244- nux::color::White,
245- true,
246- rop);
247+#ifndef NUX_OPENGLES_20
248+ if (GfxContext.UsingGLSLCodePath())
249+ gPainter.PushDrawCompositionLayer(GfxContext, geo,
250+ bg_blur_texture_,
251+ texxform_blur_bg,
252+ nux::color::White,
253+ _bg_color,
254+ nux::LAYER_BLEND_MODE_OVERLAY,
255+ true, rop);
256+ else
257+ gPainter.PushDrawTextureLayer(GfxContext, geo,
258+ bg_blur_texture_,
259+ texxform_blur_bg,
260+ nux::color::White,
261+ true,
262+ rop);
263+#else
264+ gPainter.PushDrawCompositionLayer(GfxContext, geo,
265+ bg_blur_texture_,
266+ texxform_blur_bg,
267+ nux::color::White,
268+ _bg_color,
269+ nux::LAYER_BLEND_MODE_OVERLAY,
270+ true, rop);
271+#endif
272
273 GfxContext.PopClippingRectangle();
274 }
275@@ -269,7 +288,8 @@
276
277
278
279- nux::GetPainter().RenderSinglePaintLayer(GfxContext, GetGeometry(), _bg_layer);
280+ if (_dash_is_open == false)
281+ nux::GetPainter().RenderSinglePaintLayer(GfxContext, GetGeometry(), _bg_layer);
282
283 GfxContext.PopClippingRectangle();
284
285@@ -305,21 +325,45 @@
286 rop.SrcBlend = GL_ONE;
287 rop.DstBlend = GL_ONE_MINUS_SRC_ALPHA;
288
289- gPainter.PushTextureLayer(GfxContext, geo,
290- bg_blur_texture_,
291- texxform_blur_bg,
292- nux::color::White,
293- true,
294- rop);
295+#ifndef NUX_OPENGLES_20
296+ if (GfxContext.UsingGLSLCodePath())
297+ gPainter.PushCompositionLayer(GfxContext, geo,
298+ bg_blur_texture_,
299+ texxform_blur_bg,
300+ nux::color::White,
301+ _bg_color,
302+ nux::LAYER_BLEND_MODE_OVERLAY,
303+ true,
304+ rop);
305+ else
306+ gPainter.PushTextureLayer(GfxContext, geo,
307+ bg_blur_texture_,
308+ texxform_blur_bg,
309+ nux::color::White,
310+ true,
311+ rop);
312+
313+#else
314+ gPainter.PushCompositionLayer(GfxContext, geo,
315+ bg_blur_texture_,
316+ texxform_blur_bg,
317+ nux::color::White,
318+ _bg_color,
319+ nux::LAYER_BLEND_MODE_OVERLAY,
320+ true,
321+ rop);
322+#endif
323 bgs++;
324
325 if (_dash_is_open)
326 {
327 nux::GetPainter().PushLayer(GfxContext, GetGeometry(), _bg_darken_layer_);
328+ bgs++;
329 }
330 }
331
332- gPainter.PushLayer(GfxContext, GetGeometry(), _bg_layer);
333+ if (_dash_is_open == FALSE)
334+ gPainter.PushLayer(GfxContext, GetGeometry(), _bg_layer);
335
336 if (_dash_is_open)
337 {
338
339=== modified file 'plugins/unityshell/src/UnityWindowView.cpp'
340--- plugins/unityshell/src/UnityWindowView.cpp 2012-01-30 00:44:57 +0000
341+++ plugins/unityshell/src/UnityWindowView.cpp 2012-03-13 02:55:22 +0000
342@@ -67,6 +67,8 @@
343
344
345 nux::Geometry geo_absolute = GetAbsoluteGeometry ();
346+
347+
348 if (BackgroundEffectHelper::blur_type != BLUR_NONE)
349 {
350 nux::Geometry blur_geo(geo_absolute.x, geo_absolute.y, base.width, base.height);
351@@ -85,20 +87,44 @@
352 rop.SrcBlend = GL_ONE;
353 rop.DstBlend = GL_ONE_MINUS_SRC_ALPHA;
354
355- gPainter.PushDrawTextureLayer(GfxContext, base,
356- blur_texture,
357- texxform_blur_bg,
358- nux::color::White,
359- true,
360- rop);
361+#ifndef NUX_OPENGLES_20
362+ if (GfxContext.UsingGLSLCodePath())
363+ gPainter.PushDrawCompositionLayer(GfxContext, base,
364+ blur_texture,
365+ texxform_blur_bg,
366+ nux::color::White,
367+ background_color, nux::LAYER_BLEND_MODE_OVERLAY,
368+ true, rop);
369+ else
370+ gPainter.PushDrawTextureLayer(GfxContext, base,
371+ blur_texture,
372+ texxform_blur_bg,
373+ nux::color::White,
374+ true,
375+ rop);
376+#else
377+ gPainter.PushDrawCompositionLayer(GfxContext, base,
378+ blur_texture,
379+ texxform_blur_bg,
380+ nux::color::White,
381+ background_color, nux::LAYER_BLEND_MODE_OVERLAY,
382+ true, rop);
383+#endif
384+
385 }
386 }
387
388 nux::ROPConfig rop;
389 rop.Blend = true;
390- rop.SrcBlend = GL_ONE;
391- rop.DstBlend = GL_ONE_MINUS_SRC_ALPHA;
392- gPainter.PushDrawColorLayer (GfxContext, internal_clip, background_color, false, rop);
393+
394+#ifndef NUX_OPENGLES_20
395+ if (GfxContext.UsingGLSLCodePath() == FALSE)
396+ {
397+ rop.SrcBlend = GL_ONE;
398+ rop.DstBlend = GL_ONE_MINUS_SRC_ALPHA;
399+ gPainter.PushDrawColorLayer (GfxContext, internal_clip, background_color, false, rop);
400+ }
401+#endif
402
403 // Make round corners
404 rop.Blend = true;