[Sugar-devel] [PATCH] Fixes to the record UI

Daniel Drake dsd at laptop.org
Tue Jun 8 19:07:31 EDT 2010


On 8 June 2010 13:52,  <anishmangal2002 at gmail.com> wrote:
> Fix description:
> The patch works by hiding or resizing (to size 1px by 1px) the
> widgets not required in a particular view/mode. The updateVideoComponents
> method has been modified to hide the widgets not required in a
> particular view. Widgets that can't be hidden are resized to
> 1 x 1 pixel.
>
> Additionally, this patch also fixes the naming of some variables
> (s/butt/button/g).
>
> Tested successfully on sugar-emulator-0.88, soas-mirabelle and
> xo1-f11-0.88.


Still unanswered questions from my last mail:
There was probably a reason why it moved them off screen instead of hiding them.
Do you have any idea what it is?


This is a great start but we should look towards that 'ideal fix' route.
If we are no longer moving windows offscreen, I would expect your
patch to remove a load of code that does that, but I don't see it.
Also, if you are hiding widgets, I don't see any reason to resize them to 1x1.

Comments for other reviewers: for me, the patch is now harder to
review because it contains a load of unrelated changes renaming
variables. I agree with the name changes, but perhaps best to be
careful with feedback this way.

On IRC you said that there were some issues with your old patch when
it is applied. Are they 100% resolved now?

> Signed-off-by: anishmangal2002 <anishmangal2002 at gmail.com>
> ---
>  button.py    |   10 ++--
>  model.py     |    2 +
>  p5_button.py |   30 +++++-----
>  ui.py        |  199 +++++++++++++++++++++++++++++++---------------------------
>  4 files changed, 129 insertions(+), 112 deletions(-)
>
> diff --git a/button.py b/button.py
> index 14b9700..2b0e85c 100644
> --- a/button.py
> +++ b/button.py
> @@ -62,12 +62,12 @@ class RecdButton(TrayButton, gobject.GObject):
>         return img
>
>
> -    def setButtClickedId( self, id ):
> -        self.BUTT_CLICKED_ID = id
> +    def setButtonClickedId( self, id ):
> +        self.BUTTON_CLICKED_ID = id
>
>
> -    def getButtClickedId( self ):
> -        return self.BUTT_CLICKED_ID
> +    def getButtonClickedId( self ):
> +        return self.BUTTON_CLICKED_ID
>
>
>     def setup_rollover_options( self, info ):
> @@ -105,4 +105,4 @@ class RecdButton(TrayButton, gobject.GObject):
>
>
>     def _itemCopyToClipboardCb(self, widget):
> -        self.ui.copyToClipboard( self.recd )
> \ No newline at end of file
> +        self.ui.copyToClipboard( self.recd )
> diff --git a/model.py b/model.py
> index b7f592b..e24752e 100644
> --- a/model.py
> +++ b/model.py
> @@ -323,6 +323,8 @@ class Model:
>     def startTakingPhoto( self ):
>         self.setUpdating( True )
>         self.ca.glive.takePhoto()
> +        self.ca.ui.FULLSCREEN = False
> +        self.ca.ui.updateVideoComponents()

This needs some explanation.
We can be taking photos in full screen mode.
There are also no changes in the video components when we are taking a photo.

So these 2 lines seem wrong/unnecessary or indicative of a larger problem.


> @@ -335,7 +335,7 @@ class UI:
>         self.tagsBuffer = gtk.TextBuffer()
>         self.tagsBuffer.connect('changed', self._tagsBufferEditedCb)
>         self.tagsField = gtk.TextView(self.tagsBuffer)
> -        self.tagsField.set_size_request( 100, 100 )
> +        self.tagsField.set_size_request( 50, 50 )
>         self.tagsPanel.pack_start(self.tagsField, expand=True)
>         self.infoBoxTopLeft.pack_start(self.tagsPanel, expand=True)

Why this change?

> @@ -369,7 +369,6 @@ class UI:
>         self.centered = True
>         self.setUp()
>
> -
>     def _mapEventCb( self, widget, event ):
>         #when your parent window is ready, turn on the feed of live video
>         self.liveVideoWindow.disconnect(self.MAP_EVENT_ID)
> @@ -461,9 +460,11 @@ class UI:
>         self.slowLiveVideoWindow.connect("visibility-notify-event", self._visibleNotifyCb)
>
>         self.recordWindow = RecordWindow(self)
> +        self.recordWindow.set_geometry_hints(min_width=1, min_height=1)
>         self.addToWindowStack( self.recordWindow, self.windowStack[len(self.windowStack)-1] )
>
>         self.progressWindow = ProgressWindow(self)
> +        self.progressWindow.set_geometry_hints(min_width=1, min_height=1)
>         self.addToWindowStack( self.progressWindow, self.windowStack[len(self.windowStack)-1] )
>
>         self.maxWindow = gtk.Window()
> @@ -471,9 +472,11 @@ class UI:
>         self.maxWindow.modify_bg( gtk.STATE_INSENSITIVE, Constants.colorBlack.gColor )
>         maxButton = MaxButton(self)
>         self.maxWindow.add( maxButton )
> +        self.maxWindow.set_geometry_hints(min_width=1, min_height=1)
>         self.addToWindowStack( self.maxWindow, self.windowStack[len(self.windowStack)-1] )
>
>         self.scrubWindow = ScrubberWindow(self)
> +        self.scrubWindow.set_geometry_hints(min_width=1, min_height=1)
>         self.addToWindowStack( self.scrubWindow, self.windowStack[len(self.windowStack)-1] )
>
>         self.infWindow = gtk.Window()
> @@ -492,6 +495,7 @@ class UI:
>
>
>     def _visibleNotifyCb( self, widget, event ):
> +        logging.debug("_visibleNotifyCb")
>
>         if (self.LAUNCHING):
>             return
> @@ -525,6 +529,31 @@ class UI:
>             else:
>                 self.ca.stopPipes()
>
> +    def resizeWindows( self ):
> +        if self.ca.m.MODE == Constants.MODE_AUDIO:
> +            self.maxWindow.resize( 1, 1 )
> +        else:
> +            maxDim = self.getMaxDim( self.FULLSCREEN )
> +            self.maxWindow.resize( maxDim[0], maxDim[1] )
> +
> +        if self.LIVEMODE:
> +            self.pipBgdWindow.resize( 1, 1 )
> +            self.playOggWindow.resize( 1, 1 )
> +            self.livePhotoWindow.resize( 1, 1 )
> +            self.slowLiveVideoWindow.resize( 1, 1 )
> +            self.infWindow.resize( 1, 1 )
> +            if self.ca.m.MODE == Constants.MODE_PHOTO:
> +                self.progressWindow.resize( 1, 1 )
> +        else:
> +            pgdDim = self.getPgdDim( self.FULLSCREEN )
> +            self.pipBgdWindow.resize( pgdDim[0], pgdDim[1] )
> +            self.progressWindow.resize( 1, 1 )
> +            self.recordWindow.resize( 1, 1 )
> +            if self.ca.m.MODE == Constants.MODE_PHOTO:
> +                self.playOggWindow.resize( 1, 1 )
> +                self.scrubWindow.resize( 1, 1 )
> +            elif self.ca.m.MODE == Constants.MODE_VIDEO:
> +                self.liveVideoWindow.resize( 1, 1 )
>
>     def setUpWindowsSizes( self ):
>         pipDim = self.getPipDim(False)
> @@ -597,30 +626,6 @@ class UI:
>                 if (self.HIDE_WIDGET_TIMEOUT_ID != 0):
>                     gobject.source_remove( self.HIDE_WIDGET_TIMEOUT_ID )
>
> -
> -    def hideWidgets( self ):
> -        self.moveWinOffscreen( self.maxWindow )
> -        self.moveWinOffscreen( self.pipBgdWindow )
> -        self.moveWinOffscreen( self.infWindow )
> -        self.moveWinOffscreen( self.slowLiveVideoWindow )
> -
> -        if (self.FULLSCREEN):
> -            self.moveWinOffscreen( self.recordWindow )
> -            self.moveWinOffscreen( self.progressWindow )
> -            self.moveWinOffscreen( self.scrubWindow )
> -
> -        if (self.ca.m.MODE == Constants.MODE_PHOTO):
> -            if (not self.LIVEMODE):
> -                self.moveWinOffscreen( self.liveVideoWindow )
> -        elif (self.ca.m.MODE == Constants.MODE_VIDEO):
> -            if (not self.LIVEMODE):
> -                self.moveWinOffscreen( self.liveVideoWindow )
> -        elif (self.ca.m.MODE == Constants.MODE_AUDIO):
> -            if (not self.LIVEMODE):
> -                self.moveWinOffscreen( self.liveVideoWindow )
> -        self.LAST_MODE = -1
> -
> -
>     def _mouseMightaMovedCb( self ):
>         x, y = self.ca.get_pointer()
>         passedTime = 0
> @@ -637,18 +642,6 @@ class UI:
>             self.hideWidgetsTimer = time.time()
>             passedTime = 0
>
> -        if (passedTime >= 3):
> -            if (not self.hiddenWidgets):
> -                if (self.mouseInWidget(x,y)):
> -                    self.hideWidgetsTimer = time.time()
> -                elif (self.RECD_INFO_ON):
> -                    self.hideWidgetsTimer = time.time()
> -                elif (self.UPDATE_TIMER_ID != 0):
> -                    self.hideWidgetsTimer = time.time()
> -                else:
> -                    self.hideWidgets()
> -                    self.hiddenWidgets = True
> -
>         self.mx = x
>         self.my = y
>         return True
> @@ -680,6 +673,7 @@ class UI:
>
>
>     def _mediaClickedForPlayback(self, widget, event):
> +        logging.debug("_mediaClickedForPlayback")
>         if (not self.LIVEMODE):
>             if (self.shownRecd != None):
>                 if (self.ca.m.MODE != Constants.MODE_PHOTO):
> @@ -875,18 +869,18 @@ class UI:
>         kids = self.thumbTray.get_children()
>         for i in range (0, len(kids)):
>             if (self.ca.m.UPDATING or self.ca.m.RECORDING):
> -                if (kids[i].getButtClickedId() != 0):
> -                    kids[i].disconnect( kids[i].getButtClickedId() )
> -                    kids[i].setButtClickedId(0)
> +                if (kids[i].getButtonClickedId() != 0):
> +                    kids[i].disconnect( kids[i].getButtonClickedId() )
> +                    kids[i].setButtonClickedId(0)
>             else:
> -                if (kids[i].getButtClickedId() == 0):
> -                    BUTT_CLICKED_ID = kids[i].connect( "clicked", self._thumbClicked, kids[i].recd )
> -                    kids[i].setButtClickedId(BUTT_CLICKED_ID)
> +                if (kids[i].getButtonClickedId() == 0):
> +                    BUTTON_CLICKED_ID = kids[i].connect( "clicked", self._thumbClicked, kids[i].recd )
> +                    kids[i].setButtonClickedId(BUTTON_CLICKED_ID)
>
>
>     def hideAllWindows( self ):
>         for i in range (0, len(self.windowStack)):
> -            self.moveWinOffscreen( self.windowStack[i] )
> +            self.windowStack[i].hide_all()
>
>
>     def _liveButtonReleaseCb(self, widget, event):
> @@ -1003,6 +997,7 @@ class UI:
>         self.doMouseListener( True )
>         self.showLiveVideoTags()
>         self.LAST_MODE = -1 #force an update
> +
>         self.updateVideoComponents()
>         self.resetWidgetFadeTimer()
>
> @@ -1010,7 +1005,8 @@ class UI:
>     def startLiveVideo(self, force):
>         #We need to know which window and which pipe here
>
> -        #if returning from another activity, active won't be false and needs to be to get started
> +        #if returning from another activity, active won't be false
> +        #and needs to be to get started
>         if (self.ca.glive.window == self.liveVideoWindow
>             and self.ca.props.active
>             and not force):
> @@ -1025,14 +1021,6 @@ class UI:
>         self.FULLSCREEN = not self.FULLSCREEN
>         self.updateVideoComponents()
>
> -
> -    def moveWinOffscreen( self, win ):
> -        #we move offscreen to resize or else we get flashes on screen, and setting hide() doesn't allow resize & moves
> -        offW = (gtk.gdk.screen_width() + 100)
> -        offH = (gtk.gdk.screen_height() + 100)
> -        self.smartMove(win, offW, offH)
> -
> -
>     def setImgLocDim( self, win ):
>         imgDim = self.getImgDim( self.FULLSCREEN )
>         self.smartResize( win, imgDim[0], imgDim[1] )
> @@ -1175,12 +1163,12 @@ class UI:
>             if (self.ca.m.MODE == Constants.MODE_PHOTO):
>                 return [self.vw, self.controlBarHt]
>             else:
> -                return [self.recordButtWd, self.controlBarHt]
> +                return [self.recordButtonWd, self.controlBarHt]
>         else:
>             if (self.ca.m.MODE == Constants.MODE_PHOTO):
>                 return [gtk.gdk.screen_width()-(self.inset*2), self.controlBarHt]
>             else:
> -                return [self.recordButtWd, self.controlBarHt]
> +                return [self.recordButtonWd, self.controlBarHt]
>
>
>     def getInbLoc( self, full ):
> @@ -1253,16 +1241,16 @@ class UI:
>
>     def getPrgDim( self, full ):
>         if (not full):
> -            return [self.vw-self.recordButtWd, self.controlBarHt]
> +            return [self.vw-self.recordButtonWd, self.controlBarHt]
>         else:
> -            return [gtk.gdk.screen_width()-(self.inset+self.inset+self.recordButtWd), self.controlBarHt]
> +            return [gtk.gdk.screen_width()-(self.inset+self.inset+self.recordButtonWd), self.controlBarHt]
>
>
>     def getPrgLoc( self, full ):
>         if (not full):
> -            return [self.centerBoxPos[0]+self.recordButtWd, self.centerBoxPos[1]+self.vh]
> +            return [self.centerBoxPos[0]+self.recordButtonWd, self.centerBoxPos[1]+self.vh]
>         else:
> -            return [self.inset+self.recordButtWd, gtk.gdk.screen_height()-(self.inset+self.controlBarHt)]
> +            return [self.inset+self.recordButtonWd, gtk.gdk.screen_height()-(self.inset+self.controlBarHt)]
>
>
>     def getLoc( self, pos, full ):
> @@ -1316,6 +1304,7 @@ class UI:
>             else:
>                 #or, if there is no countdown, it might be because we are recording
>                 self.clickShutter()
> +                self.progressWindow.updateProgress( 1, Constants.istrFinishedRecording )

This change is also surprising to see.

Daniel


More information about the Sugar-devel mailing list