[glom/glom-1-20] Print Layout: Avoid sometimes over-scaled images.

[Murray Cumming <murrayc src gnome org>]

commit ffdd587e750451a1ad2759c0b18116c82c2c9050
Author: Murray Cumming <murrayc murrayc com>
Date:   Wed Feb 8 11:24:59 2012 +0100

    Print Layout: Avoid sometimes over-scaled images.
    
    * glom/print_layout/canvas_layout_item.[h|cc]: Remove the constructor and
    create() that take a LayoutItem, because we should put the CanvasItem in a
    canvas (even indirectly) before creating the child item, to avoid this
    goocanvas bug: https://bugzilla.gnome.org/show_bug.cgi?id=657592#c16
    * glom/mode_design/print_layouts/window_print_layout_edit.[h|cc]:
    * glom/print_layout/canvas_print_layout.[h|cc]:
    Add and use create_canvas_layout_item_and_add() methods which put the new
    canvas item in the canvas before creating its child items based on the
    LayoutItem.
    
    This fixes bug #668901 (alien)

 ChangeLog                                          |   16 +++++++
 .../print_layouts/window_print_layout_edit.cc      |   42 +++++++++++---------
 .../print_layouts/window_print_layout_edit.h       |    2 +
 glom/print_layout/canvas_layout_item.cc            |   19 +++------
 glom/print_layout/canvas_layout_item.h             |    7 +++-
 glom/print_layout/canvas_print_layout.cc           |   24 +++++++----
 glom/print_layout/canvas_print_layout.h            |    2 +
 7 files changed, 71 insertions(+), 41 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index c514f0b..e673628 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2012-02-08  Murray Cumming  <murrayc murrayc com>
+
+	Print Layout: Avoid sometimes over-scaled images.
+
+	* glom/print_layout/canvas_layout_item.[h|cc]: Remove the constructor and
+	create() that take a LayoutItem, because we should put the CanvasItem in a 
+	canvas (even indirectly) before creating the child item, to avoid this
+	goocanvas bug: https://bugzilla.gnome.org/show_bug.cgi?id=657592#c16
+	* glom/mode_design/print_layouts/window_print_layout_edit.[h|cc]:
+	* glom/print_layout/canvas_print_layout.[h|cc]:
+	Add and use create_canvas_layout_item_and_add() methods which put the new 
+	canvas item in the canvas before creating its child items based on the 
+	LayoutItem.
+	
+	This fixes bug #668901 (alien)
+
 1.20.8:
 
 2012-02-05  Murray Cumming  <murrayc murrayc com>
diff --git a/glom/mode_design/print_layouts/window_print_layout_edit.cc b/glom/mode_design/print_layouts/window_print_layout_edit.cc
index ffe9cff..7c6616d 100644
--- a/glom/mode_design/print_layouts/window_print_layout_edit.cc
+++ b/glom/mode_design/print_layouts/window_print_layout_edit.cc
@@ -567,8 +567,8 @@ void Window_PrintLayout_Edit::on_canvas_drag_data_received(const Glib::RefPtr<Gd
       //Show it on the canvas, at the position:
       if(layout_item)
       {
-        m_layout_item_dropping = CanvasLayoutItem::create(layout_item);
-        m_canvas.add_canvas_layout_item(m_layout_item_dropping);
+        m_layout_item_dropping = 
+          create_canvas_layout_item_and_add(layout_item);
 
         m_layout_item_dropping->snap_position(item_x, item_y);
         m_layout_item_dropping->set_xy(item_x, item_y);
@@ -592,8 +592,8 @@ void Window_PrintLayout_Edit::on_canvas_drag_data_received(const Glib::RefPtr<Gd
       return;
     }
 
-    Glib::RefPtr<CanvasLayoutItem> item = CanvasLayoutItem::create(layout_item);
-    m_canvas.add_canvas_layout_item(item);
+    Glib::RefPtr<CanvasLayoutItem> item =
+      create_canvas_layout_item_and_add(layout_item);
     double item_x = x;
     double item_y = y;
     canvas_convert_from_drag_pixels(item_x, item_y, true /* adjust for scrolling */);
@@ -856,8 +856,7 @@ void Window_PrintLayout_Edit::on_menu_insert_field()
   // Note to translators: This is the default contents of a text item on a print layout: 
   set_default_position(layout_item);
 
-  Glib::RefPtr<CanvasLayoutItem> item = CanvasLayoutItem::create(layout_item);
-  m_canvas.add_canvas_layout_item(item);
+  create_canvas_layout_item_and_add(layout_item);
 }
 
 void Window_PrintLayout_Edit::on_menu_insert_text()
@@ -865,8 +864,7 @@ void Window_PrintLayout_Edit::on_menu_insert_text()
   sharedptr<LayoutItem> layout_item = create_empty_item(PrintLayoutToolbarButton::ITEM_TEXT);
   set_default_position(layout_item);
 
-  Glib::RefPtr<CanvasLayoutItem> item = CanvasLayoutItem::create(layout_item);
-  m_canvas.add_canvas_layout_item(item);
+  create_canvas_layout_item_and_add(layout_item);
 }
 
 void Window_PrintLayout_Edit::on_menu_insert_image()
@@ -876,8 +874,7 @@ void Window_PrintLayout_Edit::on_menu_insert_image()
   //layout_item->set_text(_("text"));
   set_default_position(layout_item);
 
-  Glib::RefPtr<CanvasLayoutItem> item = CanvasLayoutItem::create(layout_item);
-  m_canvas.add_canvas_layout_item(item);
+  create_canvas_layout_item_and_add(layout_item);
 }
 
 void Window_PrintLayout_Edit::on_menu_insert_relatedrecords()
@@ -885,8 +882,7 @@ void Window_PrintLayout_Edit::on_menu_insert_relatedrecords()
   sharedptr<LayoutItem> layout_item = create_empty_item(PrintLayoutToolbarButton::ITEM_PORTAL);
   set_default_position(layout_item);
 
-  Glib::RefPtr<CanvasLayoutItem> item = CanvasLayoutItem::create(layout_item);
-  m_canvas.add_canvas_layout_item(item);
+  create_canvas_layout_item_and_add(layout_item);
 }
 
 void Window_PrintLayout_Edit::on_menu_insert_line_horizontal()
@@ -903,16 +899,14 @@ void Window_PrintLayout_Edit::on_menu_insert_line_horizontal()
   //layout_item->set_text(_("text"));
   //layout_item->set_coordinates(item_x, item_y, item_x + 100, item_y);
 
-  Glib::RefPtr<CanvasLayoutItem> item = CanvasLayoutItem::create(layout_item);
-  m_canvas.add_canvas_layout_item(item);
+  create_canvas_layout_item_and_add(layout_item);
 }
 
 void Window_PrintLayout_Edit::on_menu_insert_line_vertical()
 {
   sharedptr<LayoutItem> layout_item = create_empty_item(PrintLayoutToolbarButton::ITEM_LINE_VERTICAL);
 
-  Glib::RefPtr<CanvasLayoutItem> item = CanvasLayoutItem::create(layout_item);
-  m_canvas.add_canvas_layout_item(item);
+  create_canvas_layout_item_and_add(layout_item);
 }
 
 void Window_PrintLayout_Edit::on_menu_insert_create_standard()
@@ -1138,12 +1132,22 @@ void Window_PrintLayout_Edit::on_menu_edit_paste()
     y += offset;
     item->set_print_layout_position(x, y, width, height);
 
-    Glib::RefPtr<CanvasLayoutItem> canvas_item = 
-      CanvasLayoutItem::create(item);
-    m_canvas.add_canvas_layout_item(canvas_item);
+    create_canvas_layout_item_and_add(item);
+
   }
 }
 
+Glib::RefPtr<CanvasLayoutItem> Window_PrintLayout_Edit::create_canvas_layout_item_and_add(const sharedptr<LayoutItem>& layout_item)
+{
+  Glib::RefPtr<CanvasLayoutItem> canvas_item = CanvasLayoutItem::create();
+  m_canvas.add_canvas_layout_item(canvas_item);
+  canvas_item->set_layout_item(layout_item);
+  
+  //canvas_item->set_outline_visible(m_outline_visibility);
+  
+  return canvas_item;
+}
+
 void Window_PrintLayout_Edit::on_menu_edit_delete()
 {
   while(!m_layout_items_selected.empty())
diff --git a/glom/mode_design/print_layouts/window_print_layout_edit.h b/glom/mode_design/print_layouts/window_print_layout_edit.h
index 09e0eab..b545fc2 100644
--- a/glom/mode_design/print_layouts/window_print_layout_edit.h
+++ b/glom/mode_design/print_layouts/window_print_layout_edit.h
@@ -133,6 +133,8 @@ private:
   void canvas_convert_from_drag_pixels(double& x, double& y, bool adjust_for_scrolling = false) const;
   void get_dimensions_of_multiple_selected_items(double& x, double& y, double& width, double& height);
 
+  Glib::RefPtr<CanvasLayoutItem> create_canvas_layout_item_and_add(const sharedptr<LayoutItem>& layout_item);
+
   //Box_DB_Table_Definition* m_box;
   Glib::ustring m_name_original;
   Glib::ustring m_table_name;
diff --git a/glom/print_layout/canvas_layout_item.cc b/glom/print_layout/canvas_layout_item.cc
index 8ca1a63..a3e48e5 100644
--- a/glom/print_layout/canvas_layout_item.cc
+++ b/glom/print_layout/canvas_layout_item.cc
@@ -48,14 +48,6 @@ CanvasLayoutItem::CanvasLayoutItem()
   signal_resized().connect( sigc::mem_fun(*this, &CanvasLayoutItem::on_resized) );
 }
 
-CanvasLayoutItem::CanvasLayoutItem(const sharedptr<LayoutItem>& layout_item)
-{
-  set_layout_item(layout_item);
-
-  //Rescale images when the canvas item is resized:
-  signal_resized().connect( sigc::mem_fun(*this, &CanvasLayoutItem::on_resized) );
-}
-
 CanvasLayoutItem::~CanvasLayoutItem()
 {
 }
@@ -65,11 +57,6 @@ Glib::RefPtr<CanvasLayoutItem> CanvasLayoutItem::create()
   return Glib::RefPtr<CanvasLayoutItem>(new CanvasLayoutItem());
 }
 
-Glib::RefPtr<CanvasLayoutItem> CanvasLayoutItem::create(const sharedptr<LayoutItem>& layout_item)
-{
-  return Glib::RefPtr<CanvasLayoutItem>(new CanvasLayoutItem(layout_item));
-}
-
 sharedptr<LayoutItem> CanvasLayoutItem::get_layout_item()
 {
   return m_layout_item;
@@ -127,6 +114,12 @@ void CanvasLayoutItem::on_resized()
 
 void CanvasLayoutItem::set_layout_item(const sharedptr<LayoutItem>& layout_item)
 {
+  //TODO: If we can ever avoid this the also update the CanvasLayoutItem class documentation.
+  if(!get_canvas())
+  {
+    std::cerr << G_STRFUNC << ": get_canvas() returned null. This should not be called before the CanvasLayoutItem is in a canvas due to goocanvas bug https://bugzilla.gnome.org/show_bug.cgi?id=657592#c16 ." << std::endl;
+  }
+  
   //Add the new child:
   m_layout_item = layout_item;
 
diff --git a/glom/print_layout/canvas_layout_item.h b/glom/print_layout/canvas_layout_item.h
index 7db7c08..0921f0a 100644
--- a/glom/print_layout/canvas_layout_item.h
+++ b/glom/print_layout/canvas_layout_item.h
@@ -35,12 +35,17 @@ class FieldFormatting;
 class LayoutItem_Portal;
 
 /** This has the appropriate child canvas item, depending on the type of the child LayoutItem.
+ * You should call set_layout_item() after instantiating a CanvasLayoutItem via create(),
+ * and after adding the CanvasLayoutItem to a parent CanvasItem that is already in a Goocanvas::Canvas.
+ *
+ * If the CanvasLayoutItem is not already (indirectly) in a GooCanvas::Canvas then
+ * Goocanvas::Image items will show over-scaled images, due to goocanvas bug:
+ * https://bugzilla.gnome.org/show_bug.cgi?id=657592#c16 
  */
 class CanvasLayoutItem : public CanvasGroupResizable
 {
 private:
   CanvasLayoutItem();
-  CanvasLayoutItem(const sharedptr<LayoutItem>& layout_item);
   virtual ~CanvasLayoutItem();
 
 public:
diff --git a/glom/print_layout/canvas_print_layout.cc b/glom/print_layout/canvas_print_layout.cc
index 8233104..8bd52b1 100644
--- a/glom/print_layout/canvas_print_layout.cc
+++ b/glom/print_layout/canvas_print_layout.cc
@@ -144,7 +144,9 @@ sharedptr<PrintLayout> Canvas_PrintLayout::get_print_layout()
 /*
 Glib::RefPtr<CanvasLayoutItem> Canvas_PrintLayout::create_canvas_item(const sharedptr<LayoutItem>& item)
 {
-  Glib::RefPtr<CanvasLayoutItem> result = CanvasLayoutItem::create(item);
+  Glib::RefPtr<CanvasLayoutItem> result = CanvasLayoutItem::create();
+  //TODO: Add to the canvas.
+  result->set_layout_item(item);
 
   return result;
 }
@@ -167,15 +169,22 @@ void Canvas_PrintLayout::add_layout_group_children(const sharedptr<LayoutGroup>&
     }
     else
     {
-      Glib::RefPtr<CanvasLayoutItem> canvas_item = CanvasLayoutItem::create(item);
-      if(canvas_item && canvas_item->get_child()) //get_child() returns null if the layout item was not handled.
-        add_canvas_layout_item(canvas_item);
+      create_canvas_layout_item_and_add(item);
     }
   }
 
   m_modified = true;
 }
 
+void Canvas_PrintLayout::create_canvas_layout_item_and_add(const sharedptr<LayoutItem>& layout_item)
+{
+  Glib::RefPtr<CanvasLayoutItem> canvas_item = CanvasLayoutItem::create();
+  add_canvas_layout_item(canvas_item);
+  canvas_item->set_layout_item(layout_item);
+  
+  canvas_item->set_outline_visible(m_outline_visibility);
+}
+
 void Canvas_PrintLayout::add_canvas_layout_item(const Glib::RefPtr<CanvasLayoutItem>& item)
 {
   if(!item)
@@ -193,7 +202,8 @@ void Canvas_PrintLayout::add_canvas_layout_item(const Glib::RefPtr<CanvasLayoutI
       item) );
 #endif //GLOM_ENABLE_CLIENT_ONLY
 
-   item->set_outline_visible(m_outline_visibility);
+   if(item->get_child())
+     item->set_outline_visible(m_outline_visibility);
 
    fill_with_data_system_preferences(item, get_document());
 }
@@ -213,9 +223,7 @@ void Canvas_PrintLayout::add_layout_group(const sharedptr<LayoutGroup>& group, b
   //Add the group item:
   if(!is_top_level)
   {
-    Glib::RefPtr<CanvasLayoutItem> canvas_item = CanvasLayoutItem::create(group);
-    if(canvas_item)
-      add_canvas_layout_item(canvas_item);
+    create_canvas_layout_item_and_add(group);
   }
 
   //Add the group's children.
diff --git a/glom/print_layout/canvas_print_layout.h b/glom/print_layout/canvas_print_layout.h
index 20302be..f99bf16 100644
--- a/glom/print_layout/canvas_print_layout.h
+++ b/glom/print_layout/canvas_print_layout.h
@@ -114,6 +114,8 @@ private:
   
   type_vecConstLayoutFields get_portal_fields_to_show(const sharedptr<LayoutItem_Portal>& portal);
 
+  void create_canvas_layout_item_and_add(const sharedptr<LayoutItem>& layout_item);
+
 #ifndef GLOM_ENABLE_CLIENT_ONLY
   sharedptr<LayoutItem_Portal> offer_related_records(const sharedptr<LayoutItem_Portal>& portal, Gtk::Window* parent);
   sharedptr<LayoutItem_Line> offer_line(const sharedptr<LayoutItem_Line>& portal, Gtk::Window* parent);