Merge lp:~docky-core/plank/item-foreground-buffer into lp:plank

[Rico Tzschichholz]

* Refactor the way item-layers are drawn.

* Draw item-shadows behind the dock-background. (The theme should disable the shadows if TopPadding isn't < 0)

* Puts the badge/progress-bar onto a foreground_surface owned by the dock-item. This avoids the redraw from scratch of the badge/progress-bar on every dock-redraw.

[lp:~docky-core/plank/item-foreground-buffer]

805. By Rico Tzschichholz on 2013-04-22

build: Use LT_LIB_M instead of hardcoding -lm

806. By Rico Tzschichholz on 2013-04-22

Add theme-configurable icon-shaped shadows

This adds new DockTheme preference "IconShadowSize" to adjust the shadow
with a default of 1.0

[Robert Dyer (psybers)]

Lines 9-13 - this check shouldnt be here. Always pass in the func and instead do this check inside the item itself (let the item decide if it needs to draw overlay, based on its own count/progress)

review: Needs Fixing

[Robert Dyer (psybers)]

the blocks 194-200 and 210-217 are the same, arent they? you can refactor this code to eliminate this duplication

review: Needs Fixing

[Rico Tzschichholz (ricotz)]

> Lines 9-13 - this check shouldnt be here. Always pass in the func and instead
> do this check inside the item itself (let the item decide if it needs to draw
> overlay, based on its own count/progress)

Checking it that early and passing null as func saves a cr.set_source_surface () call, same for checking the shadow-size above that.

[Rico Tzschichholz (ricotz)]

> the blocks 194-200 and 210-217 are the same, arent they? you can refactor this
> code to eliminate this duplication

It is nearly identical except for their sizes which depend on the func==null condition, an alternative might be to put it in an extra inline function.

Suggest a diff then.

[lp:~docky-core/plank/item-foreground-buffer]

807. By Rico Tzschichholz on 2013-04-24

positionmanager: Refactor get_background_position to get_background_region

This makes its usage in DockRenderer easier.

808. By Rico Tzschichholz on 2013-04-24

Use compact way to initialize structs

Note that all fields need be qualified in their defined order.

809. By Rico Tzschichholz on 2013-04-24

titledseparatormenuitem: Don't use deprecated gtk+ api

It didn't work before and doesnt completely now either.

811. By Rico Tzschichholz on 2013-04-25

Draw item-shadows separately behind the dock background

812. By Rico Tzschichholz on 2013-04-25

Refactor the item drawing

[Robert Dyer (psybers)]

review: Approve

=== modified file 'lib/DockRenderer.vala'
--- lib/DockRenderer.vala	2013-04-24 07:55:37 +0000
+++ lib/DockRenderer.vala	2013-04-25 12:50:31 +0000
@@ -40,6 +40,7 @@
 		Gdk.Rectangle background_rect;
 		DockSurface? main_buffer;
 		DockSurface? indicator_buffer;
+		DockSurface? shadow_buffer;
 		DockSurface? urgent_indicator_buffer;
 		DockSurface? urgent_glow_buffer;
 		
@@ -236,6 +237,7 @@
 			main_buffer = null;
 			background_buffer = null;
 			indicator_buffer = null;
+			shadow_buffer = null;
 			urgent_indicator_buffer = null;
 			urgent_glow_buffer = null;
 			
@@ -276,13 +278,19 @@
 			var start = new DateTime.now_local ();
 #endif
 			
+			var width = position_manager.DockWidth;
+			var height = position_manager.DockHeight;
+			
 			if (main_buffer == null) {
-				var width = position_manager.DockWidth;
-				var height = position_manager.DockHeight;
 				main_buffer = new DockSurface.with_surface (width, height, cr.get_target ());
 			}
 			
+			if (shadow_buffer == null) {
+				shadow_buffer = new DockSurface.with_surface (width, height, cr.get_target ());
+			}
+			
 			main_buffer.clear ();
+			shadow_buffer.clear ();
 			
 #if BENCHMARK
 			var start2 = new DateTime.now_local ();
@@ -316,6 +324,9 @@
 			
 			// draw the dock on the window
 			cr.set_operator (Operator.SOURCE);
+			cr.set_source_surface (shadow_buffer.Internal, x_offset, y_offset);
+			cr.paint ();
+			cr.set_operator (Operator.OVER);
 			cr.set_source_surface (main_buffer.Internal, x_offset, y_offset);
 			cr.paint ();
 			
@@ -363,6 +374,7 @@
 			unowned DragManager drag_manager = controller.drag_manager;
 			
 			unowned Context main_cr = main_buffer.Context;
+			unowned Context shadow_cr = shadow_buffer.Context;
 			var icon_size = position_manager.IconSize;
 			var shadow_size = controller.position_manager.IconShadowSize;
 			
@@ -370,12 +382,20 @@
 #if BENCHMARK
 			var start = new DateTime.now_local ();
 #endif
-			unowned DrawItemFunc? shadow_func = null;
+			var icon_surface = item.get_surface_copy (icon_size, icon_size, main_buffer);
+			unowned Context icon_cr = icon_surface.Context;
+			
+			DockSurface? icon_shadow_surface = null;
 			if (shadow_size > 0)
-				shadow_func = draw_item_shadow;
-			
-			var icon_surface = item.get_surface_copy (icon_size, icon_size, main_buffer, shadow_func);
-			unowned Context icon_cr = icon_surface.Context;
+				icon_shadow_surface = item.get_background_surface (draw_item_shadow);
+			
+			DockSurface? icon_overlay_surface = null;
+			if (item.CountVisible || item.ProgressVisible)
+				icon_overlay_surface = item.get_foreground_surface (draw_item_overlay);
+			
+			if (icon_overlay_surface != null)
+				icon_cr.set_source_surface (icon_overlay_surface.Internal, 0, 0);
+			
 #if BENCHMARK
 			var end = new DateTime.now_local ();
 			benchmark.add ("	item.get_surface time - %f ms".printf (end.difference (start) / 1000.0));
@@ -444,22 +464,6 @@
 				icon_cr.set_operator (Cairo.Operator.OVER);
 			}
 			
-			
-			// TODO put them onto a cached icon_overlay_surface for performance reasons?
-			// maybe even draw outside of the item-draw-area (considering the hover-area)
-			
-			var urgent_color = get_styled_color ();
-			urgent_color.add_hue (theme.UrgentHueShift);
-			
-			// draw item's count
-			if (item.CountVisible)
-				theme.draw_item_count (icon_surface, icon_size, urgent_color, item.Count);
-			
-			// draw item's progress
-			if (item.ProgressVisible)
-				theme.draw_item_progress (icon_surface, icon_size, urgent_color, item.Progress);
-			
-			
 			// darken the icon
 			if (darken > 0) {
 				icon_cr.rectangle (0, 0, icon_surface.Width, icon_surface.Height);
@@ -501,8 +505,14 @@
 				theme.draw_active_glow (main_buffer, background_rect, glow_rect, item.AverageIconColor, opacity, controller.prefs.Position);
 			}
 			
+			// draw the icon shadow
+			if (icon_shadow_surface != null) {
+				shadow_cr.set_source_surface (icon_shadow_surface.Internal, draw_rect.x - shadow_size, draw_rect.y - shadow_size);
+				shadow_cr.paint ();
+			}
+
 			// draw the icon
-			main_cr.set_source_surface (icon_surface.Internal, draw_rect.x - shadow_size, draw_rect.y - shadow_size);
+			main_cr.set_source_surface (icon_surface.Internal, draw_rect.x, draw_rect.y);
 			main_cr.paint ();
 			
 			// draw indicators
@@ -510,6 +520,34 @@
 				draw_indicator_state (hover_rect, item.Indicator, item.State);
 		}
 		
+		DockSurface draw_item_overlay (DockItem item, DockSurface icon_surface, DockSurface? current_surface)
+		{
+			unowned PositionManager position_manager = controller.position_manager;
+			var width = icon_surface.Width;
+			var height = icon_surface.Height;
+			
+			if (current_surface != null
+				&& width == current_surface.Width && height == current_surface.Height)
+				return current_surface;
+			
+			Logger.verbose ("DockItem.draw_item_overlay (width = %i, height = %i)", width, height);
+			var surface = new DockSurface.with_dock_surface (width, height, icon_surface);
+			
+			var icon_size = position_manager.IconSize;
+			var urgent_color = get_styled_color ();
+			urgent_color.add_hue (theme.UrgentHueShift);
+			
+			// draw item's count
+			if (item.CountVisible)
+				theme.draw_item_count (surface, icon_size, urgent_color, item.Count);
+			
+			// draw item's progress
+			if (item.ProgressVisible)
+				theme.draw_item_progress (surface, icon_size, urgent_color, item.Progress);
+			
+			return surface;
+		}
+		
 		DockSurface draw_item_shadow (DockItem item, DockSurface icon_surface, DockSurface? current_surface)
 		{
 			var shadow_size = controller.position_manager.IconShadowSize;
@@ -548,9 +586,6 @@
 			cr.paint_with_alpha (0.44);
 			surface.gaussian_blur (shadow_size);
 			
-			cr.set_source_surface (icon_surface.Internal, shadow_size, shadow_size);
-			cr.paint ();
-			
 			return surface;
 		}
 		

=== modified file 'lib/Drawing/DockSurface.vala'
--- lib/Drawing/DockSurface.vala	2013-04-19 12:37:27 +0000
+++ lib/Drawing/DockSurface.vala	2013-04-25 12:50:31 +0000
@@ -111,6 +111,22 @@
 		}
 		
 		/**
+		 * Create a copy of the surface
+		 *
+		 * @return copy of this surface
+		 */
+		public DockSurface copy ()
+		{
+			var copy = new DockSurface.with_dock_surface (Width, Height, this);
+			unowned Cairo.Context cr = copy.Context;
+			
+			cr.set_source_surface (Internal, 0, 0);
+			cr.paint ();
+			
+			return copy;
+		}
+		
+		/**
 		 * Saves the current dock surface to a {@link Gdk.Pixbuf}.
 		 *
 		 * @return the {@link Gdk.Pixbuf}

=== modified file 'lib/Items/DockItem.vala'
--- lib/Items/DockItem.vala	2013-04-22 13:19:32 +0000
+++ lib/Items/DockItem.vala	2013-04-25 12:50:31 +0000
@@ -29,7 +29,7 @@
 	 * Draws a modified surface onto another newly created or given surface
 	 *
 	 * @param item the dock-item
-	 * @param source original surface
+	 * @param source original surface which may not be changed
 	 * @param target the previously modified surface
 	 * @return the modified surface or passed through target
 	 */
@@ -289,6 +289,7 @@
 		
 		DockSurface? surface = null;
 		DockSurface? background_surface = null;
+		DockSurface? foreground_surface = null;
 		
 		/**
 		 * Creates a new dock item.
@@ -304,6 +305,11 @@
 			Gtk.IconTheme.get_default ().changed.connect (icon_theme_changed);
 			notify["Icon"].connect (reset_icon_buffer);
 			notify["ForcePixbuf"].connect (reset_icon_buffer);
+			
+			notify["Count"].connect (reset_foreground_buffer);
+			notify["CountVisible"].connect (reset_foreground_buffer);
+			notify["Progress"].connect (reset_foreground_buffer);
+			notify["ProgressVisible"].connect (reset_foreground_buffer);
 		}
 		
 		~DockItem ()
@@ -312,6 +318,11 @@
 			Gtk.IconTheme.get_default ().changed.disconnect (icon_theme_changed);
 			notify["Icon"].disconnect (reset_icon_buffer);
 			notify["ForcePixbuf"].disconnect (reset_icon_buffer);
+			
+			notify["Count"].disconnect (reset_foreground_buffer);
+			notify["CountVisible"].disconnect (reset_foreground_buffer);
+			notify["Progress"].disconnect (reset_foreground_buffer);
+			notify["ProgressVisible"].disconnect (reset_foreground_buffer);
 		}
 		
 		/**
@@ -337,6 +348,7 @@
 		{
 			surface = null;
 			background_surface = null;
+			foreground_surface = null;
 			
 			needs_redraw ();
 		}
@@ -347,6 +359,12 @@
 		public void reset_buffers ()
 		{
 			background_surface = null;
+			foreground_surface = null;
+		}
+		
+		void reset_foreground_buffer ()
+		{
+			foreground_surface = null;
 		}
 		
 		void icon_theme_changed ()
@@ -359,7 +377,7 @@
 			});
 		}
 		
-		DockSurface get_surface (int width, int height, DockSurface model)
+		unowned DockSurface get_surface (int width, int height, DockSurface model)
 		{
 			if (surface == null || width != surface.Width || height != surface.Height) {
 				surface = new DockSurface.with_dock_surface (width, height, model);
@@ -374,38 +392,63 @@
 		}
 		
 		/**
-		 * Returns a copy of the dock surface for this item and triggers an
-		 * internal redraw if the requested size isn't matching the cache.
-		 *
-		 * @param width width of the requested surface
-		 * @param height height of the requested surface
+		 * Returns the background surface for this item.
+		 *
+		 * The draw_func may pass through the given previously computed surface
+		 * or change it as needed. This surface will be buffered internally.
+		 *
+		 * Passing null as draw_func will destroy the internal background buffer.
+		 *
+		 * @param draw_func function which creates/changes the background surface
+		 * @return the background surface of this item which may not be changed
+		 */
+		public unowned DockSurface? get_background_surface (DrawItemFunc? draw_func = null)
+			requires (surface != null)
+		{
+			if (draw_func != null)
+				background_surface = draw_func (this, surface, background_surface);
+			else
+				background_surface = null;
+			
+			return background_surface;
+		}
+		
+		/**
+		 * Returns the foreground surface for this item.
+		 *
+		 * The draw_func may pass through the given previously computed surface
+		 * or change it as needed. This surface will be buffered internally.
+		 *
+		 * Passing null as draw_func will destroy the internal foreground buffer.
+		 *
+		 * @param draw_func function which creates/changes the foreground surface
+		 * @return the background surface of this item which may not be changed
+		 */
+		public unowned DockSurface? get_foreground_surface (DrawItemFunc? draw_func = null)
+			requires (surface != null)
+		{
+			if (draw_func != null)
+				foreground_surface = draw_func (this, surface, foreground_surface);
+			else
+				foreground_surface = null;
+			
+			return foreground_surface;
+		}
+		
+		/**
+		 * Returns a copy of the dock surface for this item.
+		 *
+		 * It will trigger an internal redraw if the requested size
+		 * isn't matching the cache.
+		 *
+		 * @param width width of the icon surface
+		 * @param height height of the icon surface
 		 * @param model existing surface to use as basis of new surface
-		 * @param draw_func function which manipulates the resulting surface
 		 * @return the copied dock surface for this item
 		 */
-		public DockSurface get_surface_copy (int width, int height, DockSurface model, DrawItemFunc? draw_func = null)
+		public DockSurface get_surface_copy (int width, int height, DockSurface model)
 		{
-			var icon_surface = get_surface (width, height, model);
-			
-			if (draw_func == null) {
-				var surface_copy = new DockSurface.with_dock_surface (width, height, model);
-				unowned Cairo.Context cr = surface_copy.Context;
-				
-				cr.set_source_surface (icon_surface.Internal, 0, 0);
-				cr.paint ();
-			
-				return surface_copy;
-			}
-			
-			background_surface = draw_func (this, icon_surface, background_surface);
-			
-			var surface_copy = new DockSurface.with_dock_surface (background_surface.Width, background_surface.Height, model);
-			unowned Cairo.Context cr = surface_copy.Context;
-			
-			cr.set_source_surface (background_surface.Internal, 0, 0);
-			cr.paint ();
-			
-			return surface_copy;
+			return get_surface (width, height, model).copy ();
 		}
 
 		/**