Opened 11 years ago

Closed 7 years ago

Last modified 7 years ago

#1106 closed defect (fixed)

Browse: No preview in Journal for downloaded image

Reported by: Mokurai Owned by: manuq
Priority: Normal Milestone:
Component: Browse Version: Unspecified
Severity: Minor Keywords: GPA, regression, 12.1.0, olpc-test-pending
Cc: alsroot, humitos, manuq Distribution/OS:
Bug Status: Unconfirmed

Description

When I right-click an image in Browse and download it, there is no preview in the Journal entry. If I then open the image in Record and immediately quit, a preview appears.

Soas Strawberry in VirtualBox OSE.

Attachments (1)

0001-Fix-1106-No-preview-in-Journal-for-downloaded-image.patch (2.9 KB) - added by godiard 10 years ago.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 11 years ago by walter

The short term workaround is to open the image in ImageViewer, which will then generate a preview (and give you the opportunity to change the title, which is most likely something pretty unfriendly and add some tags and a description.)

comment:2 Changed 10 years ago by gregorio

  • Keywords GPA added

I think is important and would be very helpful per GPA experience.

e.g.: "- Weren't sure which item in journal was the state image they just downloaded."

from: http://www.mail-archive.com/sugar-devel@lists.sugarlabs.org/msg06470.html

Thanks,

Greg S

comment:3 Changed 10 years ago by tomeu

  • Component changed from sugar to journal
  • Milestone changed from Unspecified by Release Team to 0.88
  • Priority changed from Unspecified by Maintainer to High
  • Severity changed from Unspecified to Critical
  • Type changed from defect to enhancement

comment:4 Changed 10 years ago by walter

  • Milestone changed from 0.88 to 0.90

Yet another Journal ticket to work on for 0.90

comment:5 follow-ups: Changed 10 years ago by godiard

This patch resolves the problem in browser.
I don't know if may be a good idea do the preview in the datastore instead.

Gonzalo

[gonzalo@aronax Browse.activity]$ diff -u downloadmanager.py.ori downloadmanager.py
--- downloadmanager.py.ori    2010-07-05 20:27:06.000000000 -0300
+++ downloadmanager.py    2010-08-06 02:17:23.095272320 -0300
@@ -36,8 +36,10 @@
 from sugar import mime
 from sugar.graphics.alert import Alert, TimeoutAlert
 from sugar.graphics.icon import Icon
+from sugar.graphics import style
 from sugar.activity import activity
 
+
 # #3903 - this constant can be removed and assumed to be 1 when dbus-python
 # 0.82.3 is the only version used
 import dbus
@@ -192,12 +194,47 @@
                 sniffed_mime_type = mime.get_for_file(self._target_file.path)
                 self.dl_jobject.metadata['mime_type'] = sniffed_mime_type
 
+            if self._mime_type in ('image/bmp','image/gif','image/jpeg','image/png','image/tiff'):
+
+                self.dl_jobject.metadata['preview'] = self.__get_preview_image()
+            else:
+                self.dl_jobject.metadata['preview'] = ''
+
             datastore.write(self.dl_jobject,
                             transfer_ownership=True,
                             reply_handler=self._internal_save_cb,
                             error_handler=self._internal_save_error_cb,
                             timeout=360 * DBUS_PYTHON_TIMEOUT_UNITS_PER_SECOND)
 
+    def __get_preview_image(self):
+        pixbuf = gtk.gdk.pixbuf_new_from_file(self._target_file.path)
+        width, height = pixbuf.get_width(), pixbuf.get_height()
+
+        preview_width = style.zoom(300)
+        preview_height =  style.zoom(225)
+
+        if (width > preview_width) or (height > preview_height):
+            scale_x = float(width) / preview_width
+            scale_y = float(height) / preview_height
+            scale = max(scale_x,scale_y)
+       
+            pixbuf = pixbuf.scale_simple(float(width) / scale, height / scale,
+                                     gtk.gdk.INTERP_BILINEAR)
+        pixbuf2 = gtk.gdk.Pixbuf(gtk.gdk.COLORSPACE_RGB,pixbuf.get_has_alpha(),8,preview_width,preview_height)
+        pixbuf2.fill(0xffffffff)
+        margin_x = (preview_width - pixbuf.get_width()) / 2
+        margin_y = (preview_height - pixbuf.get_height()) / 2
+
+        pixbuf.copy_area(0,0,pixbuf.get_width(),pixbuf.get_height(),pixbuf2,margin_x,margin_y)
+
+        preview_data = []
+        def save_func(buf, data):
+            data.append(buf)
+
+        pixbuf2.save_to_callback(save_func, 'png', user_data=preview_data)
+        preview_data = ''.join(preview_data)
+        return dbus.ByteArray(preview_data)
+
     def __start_response_cb(self, alert, response_id):
         global _active_downloads
         if response_id is gtk.RESPONSE_CANCEL:
@@ -273,10 +310,10 @@
         self.dl_jobject.metadata['progress'] = '0'
         self.dl_jobject.metadata['keep'] = '0'
         self.dl_jobject.metadata['buddies'] = ''
-        self.dl_jobject.metadata['preview'] = ''
         self.dl_jobject.metadata['icon-color'] = \
                 profile.get_color().to_string()
         self.dl_jobject.metadata['mime_type'] = self._mime_type
+        self.dl_jobject.metadata['preview'] = ''
         self.dl_jobject.file_path = ''
         datastore.write(self.dl_jobject)

comment:6 in reply to: ↑ 5 Changed 10 years ago by tomeu

  • Component changed from journal to Browse
  • Keywords r? added

Replying to godiard:

This patch resolves the problem in browser.
I don't know if may be a good idea do the preview in the datastore instead.

I think we should try to keep the DS as simple as possible because it's a so critical component. Also, the right size and format of previews are dependent on the UX and the DS should have no knowledge of it.

Would be great if you could send commit patches such as those generated with git format-patch, more details in http://wiki.sugarlabs.org/go/Development_Team/Code_Review#Proposal

Gonzalo

[gonzalo@aronax Browse.activity]$ diff -u downloadmanager.py.ori downloadmanager.py
--- downloadmanager.py.ori    2010-07-05 20:27:06.000000000 -0300
+++ downloadmanager.py    2010-08-06 02:17:23.095272320 -0300
@@ -36,8 +36,10 @@
 from sugar import mime
 from sugar.graphics.alert import Alert, TimeoutAlert
 from sugar.graphics.icon import Icon
+from sugar.graphics import style
 from sugar.activity import activity
 
+
 # #3903 - this constant can be removed and assumed to be 1 when dbus-python
 # 0.82.3 is the only version used
 import dbus
@@ -192,12 +194,47 @@
                 sniffed_mime_type = mime.get_for_file(self._target_file.path)
                 self.dl_jobject.metadata['mime_type'] = sniffed_mime_type
 
+            if self._mime_type in ('image/bmp','image/gif','image/jpeg','image/png','image/tiff'):
+
+                self.dl_jobject.metadata['preview'] = self.__get_preview_image()
+            else:
+                self.dl_jobject.metadata['preview'] = ''
+
             datastore.write(self.dl_jobject,
                             transfer_ownership=True,
                             reply_handler=self._internal_save_cb,
                             error_handler=self._internal_save_error_cb,
                             timeout=360 * DBUS_PYTHON_TIMEOUT_UNITS_PER_SECOND)
 
+    def __get_preview_image(self):
+        pixbuf = gtk.gdk.pixbuf_new_from_file(self._target_file.path)
+        width, height = pixbuf.get_width(), pixbuf.get_height()
+
+        preview_width = style.zoom(300)
+        preview_height =  style.zoom(225)
+
+        if (width > preview_width) or (height > preview_height):
+            scale_x = float(width) / preview_width
+            scale_y = float(height) / preview_height
+            scale = max(scale_x,scale_y)
+       
+            pixbuf = pixbuf.scale_simple(float(width) / scale, height / scale,
+                                     gtk.gdk.INTERP_BILINEAR)
+        pixbuf2 = gtk.gdk.Pixbuf(gtk.gdk.COLORSPACE_RGB,pixbuf.get_has_alpha(),8,preview_width,preview_height)
+        pixbuf2.fill(0xffffffff)
+        margin_x = (preview_width - pixbuf.get_width()) / 2
+        margin_y = (preview_height - pixbuf.get_height()) / 2
+
+        pixbuf.copy_area(0,0,pixbuf.get_width(),pixbuf.get_height(),pixbuf2,margin_x,margin_y)
+
+        preview_data = []
+        def save_func(buf, data):
+            data.append(buf)
+
+        pixbuf2.save_to_callback(save_func, 'png', user_data=preview_data)
+        preview_data = ''.join(preview_data)
+        return dbus.ByteArray(preview_data)
+
     def __start_response_cb(self, alert, response_id):
         global _active_downloads
         if response_id is gtk.RESPONSE_CANCEL:
@@ -273,10 +310,10 @@
         self.dl_jobject.metadata['progress'] = '0'
         self.dl_jobject.metadata['keep'] = '0'
         self.dl_jobject.metadata['buddies'] = ''
-        self.dl_jobject.metadata['preview'] = ''
         self.dl_jobject.metadata['icon-color'] = \
                 profile.get_color().to_string()
         self.dl_jobject.metadata['mime_type'] = self._mime_type
+        self.dl_jobject.metadata['preview'] = ''
         self.dl_jobject.file_path = ''
         datastore.write(self.dl_jobject)

comment:7 Changed 10 years ago by erikos

  • Cc alsroot added

Sorting that out would be a valuable task, thanks for working on it. When we did want to do the thumbsview in the Journal we already raised the issue. Would be great to sort it out.

comment:8 Changed 10 years ago by tomeu

  • Owner changed from tomeu to lucian
  • Status changed from new to assigned

comment:9 in reply to: ↑ 5 Changed 10 years ago by lucian

  • Priority changed from High to Normal

The patch doesn't apply: fatal: corrupt patch at line 16

Please follow Tomeu's instructions.

Replying to godiard:

This patch resolves the problem in browser.
I don't know if may be a good idea do the preview in the datastore instead.

Gonzalo

[gonzalo@aronax Browse.activity]$ diff -u downloadmanager.py.ori downloadmanager.py
--- downloadmanager.py.ori    2010-07-05 20:27:06.000000000 -0300
+++ downloadmanager.py    2010-08-06 02:17:23.095272320 -0300
@@ -36,8 +36,10 @@
 from sugar import mime
 from sugar.graphics.alert import Alert, TimeoutAlert
 from sugar.graphics.icon import Icon
+from sugar.graphics import style
 from sugar.activity import activity
 
+
 # #3903 - this constant can be removed and assumed to be 1 when dbus-python
 # 0.82.3 is the only version used
 import dbus
@@ -192,12 +194,47 @@
                 sniffed_mime_type = mime.get_for_file(self._target_file.path)
                 self.dl_jobject.metadata['mime_type'] = sniffed_mime_type
 
+            if self._mime_type in ('image/bmp','image/gif','image/jpeg','image/png','image/tiff'):
+
+                self.dl_jobject.metadata['preview'] = self.__get_preview_image()
+            else:
+                self.dl_jobject.metadata['preview'] = ''
+
             datastore.write(self.dl_jobject,
                             transfer_ownership=True,
                             reply_handler=self._internal_save_cb,
                             error_handler=self._internal_save_error_cb,
                             timeout=360 * DBUS_PYTHON_TIMEOUT_UNITS_PER_SECOND)
 
+    def __get_preview_image(self):
+        pixbuf = gtk.gdk.pixbuf_new_from_file(self._target_file.path)
+        width, height = pixbuf.get_width(), pixbuf.get_height()
+
+        preview_width = style.zoom(300)
+        preview_height =  style.zoom(225)
+
+        if (width > preview_width) or (height > preview_height):
+            scale_x = float(width) / preview_width
+            scale_y = float(height) / preview_height
+            scale = max(scale_x,scale_y)
+       
+            pixbuf = pixbuf.scale_simple(float(width) / scale, height / scale,
+                                     gtk.gdk.INTERP_BILINEAR)
+        pixbuf2 = gtk.gdk.Pixbuf(gtk.gdk.COLORSPACE_RGB,pixbuf.get_has_alpha(),8,preview_width,preview_height)
+        pixbuf2.fill(0xffffffff)
+        margin_x = (preview_width - pixbuf.get_width()) / 2
+        margin_y = (preview_height - pixbuf.get_height()) / 2
+
+        pixbuf.copy_area(0,0,pixbuf.get_width(),pixbuf.get_height(),pixbuf2,margin_x,margin_y)
+
+        preview_data = []
+        def save_func(buf, data):
+            data.append(buf)
+
+        pixbuf2.save_to_callback(save_func, 'png', user_data=preview_data)
+        preview_data = ''.join(preview_data)
+        return dbus.ByteArray(preview_data)
+
     def __start_response_cb(self, alert, response_id):
         global _active_downloads
         if response_id is gtk.RESPONSE_CANCEL:
@@ -273,10 +310,10 @@
         self.dl_jobject.metadata['progress'] = '0'
         self.dl_jobject.metadata['keep'] = '0'
         self.dl_jobject.metadata['buddies'] = ''
-        self.dl_jobject.metadata['preview'] = ''
         self.dl_jobject.metadata['icon-color'] = \
                 profile.get_color().to_string()
         self.dl_jobject.metadata['mime_type'] = self._mime_type
+        self.dl_jobject.metadata['preview'] = ''
         self.dl_jobject.file_path = ''
         datastore.write(self.dl_jobject)

comment:10 Changed 10 years ago by lucian

  • Status changed from assigned to accepted

comment:11 Changed 10 years ago by godiard

New patch:

[gonzalo@nautilus mainline]$ cat 0001-Fix-1106-No-preview-in-Journal-for-downloaded-image.patch
From 45e053104de9671c06a732a8e4dc172db2df8a56 Mon Sep 17 00:00:00 2001
From: Gonzalo Odiard <godiard@…>
Date: Mon, 16 Aug 2010 18:48:20 -0300
Subject: [PATCH] Fix #1106 No preview in Journal for downloaded image

---

downloadmanager.py | 35 +++++++++++++++++++++++++++++++++++
1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/downloadmanager.py b/downloadmanager.py
index 3eec649..0538049 100644
--- a/downloadmanager.py
+++ b/downloadmanager.py
@@ -36,6 +36,7 @@ from sugar import profile

from sugar import mime
from sugar.graphics.alert import Alert, TimeoutAlert
from sugar.graphics.icon import Icon

+from sugar.graphics import style

from sugar.activity import activity


# #3903 - this constant can be removed and assumed to be 1 when dbus-python

@@ -192,12 +193,46 @@ class Download:

sniffed_mime_type = mime.get_for_file(self._target_file.path)
self.dl_jobject.metadatamime_type? = sniffed_mime_type


+ if self._mime_type in ('image/bmp','image/gif','image/jpeg','image/png','image/tiff'):
+ self.dl_jobject.metadatapreview? = self.get_preview_image()
+ else:
+ self.dl_jobject.metadatapreview? =
+

datastore.write(self.dl_jobject,

transfer_ownership=True,
reply_handler=self._internal_save_cb,
error_handler=self._internal_save_error_cb,
timeout=360 * DBUS_PYTHON_TIMEOUT_UNITS_PER_SECOND)


+ def get_preview_image(self):
+ pixbuf = gtk.gdk.pixbuf_new_from_file(self._target_file.path)
+ width, height = pixbuf.get_width(), pixbuf.get_height()
+
+ preview_width = style.zoom(300)
+ preview_height = style.zoom(225)
+
+ if (width > preview_width) or (height > preview_height):
+ scale_x = float(width) / preview_width
+ scale_y = float(height) / preview_height
+ scale = max(scale_x,scale_y)
+
+ pixbuf = pixbuf.scale_simple(float(width) / scale, height / scale,
+ gtk.gdk.INTERP_BILINEAR)
+ pixbuf2 = gtk.gdk.Pixbuf(gtk.gdk.COLORSPACE_RGB,pixbuf.get_has_alpha(),8,preview_width,preview_height)
+ pixbuf2.fill(0xffffffff)
+ margin_x = (preview_width - pixbuf.get_width()) / 2
+ margin_y = (preview_height - pixbuf.get_height()) / 2
+
+ pixbuf.copy_area(0,0,pixbuf.get_width(),pixbuf.get_height(),pixbuf2,margin_x,margin_y)
+
+ preview_data = []
+ def save_func(buf, data):
+ data.append(buf)
+
+ pixbuf2.save_to_callback(save_func, 'png', user_data=preview_data)
+ preview_data = .join(preview_data)
+ return dbus.ByteArray(preview_data)
+

def start_response_cb(self, alert, response_id):

global _active_downloads
if response_id is gtk.RESPONSE_CANCEL:

--
1.7.2

comment:12 Changed 10 years ago by lucian

The patch doesn't apply again (fatal: corrupt patch at line 13). Please create your patch with git following Tomeu's instructions and upload a .patch file instead of pasting it in the comment.

comment:13 Changed 9 years ago by martin.langhoff

Bump - review request.

This time the patch is well formatted. git whines a bit about a bit of trailing whitespace so I did git am --whitespace=strip 0001-Fix-1106-No-preview-in-Journal-for-downloaded-image.patch

comment:14 Changed 9 years ago by sascha_silbe

  • Distribution/OS Unspecified deleted
  • Keywords r? removed
  • Severity changed from Critical to Minor

The code looks rather complex for scaling and saving an image. Have you tried using Pixbuf.composite() / Pixbuf.composite_colors() or a different library (e.g. cairo or PIL)?

For the next iteration please add a patch description and post the patch on sugar-devel using git send-email.

comment:15 Changed 9 years ago by lucian

  • Type changed from enhancement to defect

Looks ok for now and seems to work.

This has been in patch limbo for quite a while, so I'll have a look to see if I can make it simpler. If I can't figure something out soon, I'll leave this for 0.92.

RFC

comment:16 Changed 9 years ago by godiard

  • Type changed from defect to enhancement

Please review the last version of the patch http://patchwork.sugarlabs.org/patch/273/

comment:17 Changed 9 years ago by godiard

  • Resolution set to fixed
  • Status changed from accepted to closed

comment:18 Changed 9 years ago by sascha_silbe

  • Resolution fixed deleted
  • Status changed from closed to reopened

The plan was to keep the ticket open so we'll take another look when we have time. The link to the commit is missing as well.

comment:20 Changed 9 years ago by RafaelOrtiz

  • Resolution set to fixed
  • Status changed from reopened to closed

I think this should be closed for now, as per Sascha's comment should be improvements for the patch, but the main issue is solved.


comment:21 Changed 8 years ago by humitos

  • Cc humitos manuq added
  • Keywords regression 12.1.0 added
  • Milestone changed from 0.90 to 0.96
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Type changed from enhancement to defect

I think this feature was lost when Browse was ported to Gtk3 because it's no longer working.

comment:22 Changed 8 years ago by manuq

  • Owner changed from lucian to manuq
  • Status changed from reopened to assigned

comment:23 Changed 7 years ago by manuq

  • Milestone changed from 0.96 to 1.0

comment:24 Changed 7 years ago by manuq

  • Keywords olpc-test-pending added
  • Resolution set to fixed
  • Status changed from assigned to closed

I have pushed a new implementation based on cairo and GdkPixbuf as 7e31cfa6 .

comment:25 Changed 7 years ago by dnarvaez

  • Milestone 1.0 deleted

Milestone 1.0 deleted

Note: See TracTickets for help on using tickets.