Opened 6 years ago

Closed 5 years ago

#8335 closed defect (fixed)

Colors-3 does not stop / quit

Reported by: martin.langhoff Owned by: wadeb
Priority: normal Milestone: 8.2.0 (was Update.2)
Component: colors-activity Version: not specified
Keywords: Colors! 8.2-759 Cc: aly
Blocked By: Blocking:
Deployments affected: Action Needed: never set
Verified: no

Description

1 - Start Colors
2 - Draw
3 - Attempt to stop / Quit hitting th stop icon

The activity does not stop. Reading the logs (attached) seems to indicate a bad interaction with the 'take a screenshot at stop time' changes in Sugar.

This is with OS 8.2-759

Attachments (1)

org.laptop.community.Colors-1.log (11.6 KB) - added by martin.langhoff 6 years ago.

Download all attachments as: .zip

Change History (17)

Changed 6 years ago by martin.langhoff

comment:1 Changed 6 years ago by rhoward

This problem also happens with Colors! V4 and OS build 8.2-759.

I tried this a few days ago with Colors! V4 and OS build 8.2-757 and you could stop Colors! successfully.

comment:2 Changed 6 years ago by rhoward

  • Keywords Colors! 8.2-759 added; colors 8 2-759 removed

comment:3 Changed 6 years ago by rhoward

Colors! V4 problem is a slight variation. Can close if you save and then use the stop from the menu on the icon in the frame.

comment:4 Changed 6 years ago by marco

  • Owner changed from mstone to wadeb

SugarPreview take_screenshot changed API. We are passing the widget in to be able to get the screen. Doeesn't the default Activity implementation work for Colors?

comment:5 Changed 6 years ago by gregorio

  • Milestone changed from Not Triaged to 8.2.0 (was Update.2)

comment:6 Changed 6 years ago by martin.langhoff

The author of Colors indicated that there's a Colors-4 that according to him fixes the problem. It wasn't listed in the Activities page when we undertook our testing (apologies!)

comment:7 Changed 6 years ago by wadeb

I'm not sure why I needed to override the screenshot code - I'll look into it and see if the overloaded method can be removed.

Note- Would be nice to have a formal notification system for when these API changes happen.

Cheers,
Wade

comment:8 follow-up: Changed 6 years ago by marco

Yeah, sorry. For 0.84 we will create a wiki page where we'll note all the API changes.

There is the question of what is private and what is public in Activity. take_screenshot is clearly public, but I'm not sure about _preview. Since we subclass Activity the usual underscore convention doesn't make it clear. If anyone has thoughts or suggestions about this I'd be glad to hear them.

comment:9 in reply to: ↑ description Changed 6 years ago by tabitha

Replying to martin.langhoff:

1 - Start Colors
2 - Draw
3 - Attempt to stop / Quit hitting th stop icon

The activity does not stop. Reading the logs (attached) seems to indicate a bad interaction with the 'take a screenshot at stop time' changes in Sugar.

This is with OS 8.2-759

Confirming this is still happening in Colors-4 testing on same XOs as original bug report

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

Replying to marco:

There is the question of what is private and what is public in Activity. take_screenshot is clearly public, but I'm not sure about _preview. Since we subclass Activity the usual underscore convention doesn't make it clear. If anyone has thoughts or suggestions about this I'd be glad to hear them.

I think that anything with an initial underscore needs to be considered as an implementation detail and thus subject to change. Ideally, all identifiers that are part of a public API wouldn't start with an underscore and would have a docstring explaining what the caller can depend on.

comment:11 Changed 6 years ago by DanKrejsa

Hi,

Here's a patch that seems to make quitting the Colors! (version 4)
activity work on candidate-765. I don't know how this change would
affect operation on older XO builds.


[dlkrejsa@localhost Colors.activity]$ diff -u colors.py ../colors.py
--- colors.py 2008-05-09 03:16:10.000000000 -0700
+++ ../colors.py 2008-09-28 11:48:37.000000000 -0700
@@ -1487,7 +1487,7 @@

def take_screenshot (self):

if self.easelarea and self.easelarea.bin_window:

  • self._preview.take_screenshot(self.easelarea.bin_window)

+ self._preview.take_screenshot(self.easelarea)

def save_thumbnail(self, filename):

pbuf = gtk.gdk.Pixbuf(gtk.gdk.COLORSPACE_RGB, False, 8,

self.width, self.height)


A couple of other questions on Colors!

  • Is it supposed to play back the saved painting automatically

when an instance starts? Currently it does not.

  • After manually going to the watch tab and playing back

the saved painting, then returning to the Paint tab, the
first stroke fails to apply any paint. The second stroke
succeeds.

comment:12 Changed 6 years ago by DanKrejsa

Sorry about the poor wiki formatting of that diff, here it is again (hope this works):

[dlkrejsa@localhost Colors.activity]$ diff -u colors.py ../colors.py 
--- colors.py   2008-05-09 03:16:10.000000000 -0700
+++ ../colors.py        2008-09-28 11:48:37.000000000 -0700
@@ -1487,7 +1487,7 @@
 
     def take_screenshot (self):
         if self.easelarea and self.easelarea.bin_window:
-            self._preview.take_screenshot(self.easelarea.bin_window)
+           self._preview.take_screenshot(self.easelarea)
 
     def save_thumbnail(self, filename):
         pbuf = gtk.gdk.Pixbuf(gtk.gdk.COLORSPACE_RGB, False, 8,
self.width, self.height)

comment:13 Changed 6 years ago by wadeb

Applied the patch, thanks! I don't think it will hurt older builds.

comment:14 Changed 6 years ago by wadeb

Also, the other bugs you reported should be fixed. They appear to have been caused by changes to Sugar, but I think I've worked around them. Can you try out:

http://www.wadeb.com/Colors!-8.xo

and close this bug if everything looks good? Then I'll release this version to the public.

Thanks for the report,

Wade

comment:15 Changed 5 years ago by cjl

  • Component changed from other-activity to colors-activity

changing component to improve discoverability

comment:16 Changed 5 years ago by martin.langhoff

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

This has long been fixed -- apologies, and thanks Wade! :-)

Note: See TracTickets for help on using tickets.