After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 696149 - Double refresh of display introduced with default unallocated space selection
Double refresh of display introduced with default unallocated space selection
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
0.15.0
Other Linux
: Normal minor
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2013-03-19 16:48 UTC by Curtis Gedak
Modified: 2014-10-20 17:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Flashing refresh investigation 1 (1.02 KB, patch)
2013-03-19 23:05 UTC, Mike Fleetwood
none Details | Review
Fix flashing of devices combobox, method 1 (draft 1) (4.11 KB, patch)
2014-08-28 21:23 UTC, Mike Fleetwood
none Details | Review
Fix flashing of devices combobox, method 2 (draft 1) (3.56 KB, patch)
2014-08-28 21:25 UTC, Mike Fleetwood
none Details | Review
Fix flashing redraw (v1) (6.08 KB, patch)
2014-10-02 13:46 UTC, Mike Fleetwood
none Details | Review
Fix flashing redraw (v2) (6.45 KB, patch)
2014-10-03 07:46 UTC, Mike Fleetwood
none Details | Review

Description Curtis Gedak 2013-03-19 16:48:11 UTC
With the enhancement to select the largest unallocated space by default, a minor new double refresh or flicker problem was created.

For more details, see:

Bug #667365 - Free space should be selected by default
Comment 1 Mike Fleetwood 2013-03-19 23:05:30 UTC
Created attachment 239314 [details] [review]
Flashing refresh investigation 1

Hi Curtis,

I'd be pleased if you could test this investigative patch and report
your findings.  I want to know that it happens on your machine too.
Also if you have and ideas about how to debug further, they would help.
It appears to need to understand the Gtk drawing and refresh model
better, which I have no clue about at the moment.

It adds 1 second pauses into Gtk event loop.  When, and only when,
unallocated free space is automatically selected the disk graphic and
partition list flash to background colour before being redrawn a few
seconds later.  Try a few refreshes and queuing and clearing operations.

P.S.  Recommend using the keyboard as it generates less events for the
GTK event loop to process than the mouse, so there will be less 1 second
waits.

Thanks,
Mike
Comment 2 Mike Fleetwood 2013-04-04 21:31:06 UTC
Hi Curtis, have you got time to look at comment #1 above?

Thanks, Mike
Comment 3 Curtis Gedak 2013-04-05 16:01:50 UTC
Hi Mike,

My apologies.  I have this on my todo list but had not got around to it.  I will look at this on the weekend.

Curtis
Comment 4 Curtis Gedak 2013-04-06 20:54:16 UTC
Hi Mike,

Using your patch from comment #1 I do notice what appears to be two separate screen drawing actions after I apply a "Create new partition" operation and close the apply dialog window.

The first refresh displays the updated partition layout graphic and the text list of partitions below.  At this time the device selection drop-down list is shown as empty.  For example it contains "", e.g., no text.

The second refresh appears to fill in the device selection drop-down list.  For example it now displays "/dev/sdd (149.05 GiB)".  Please note that I do not see any flicker on my Intel HD 3000 graphics (Intel iCore i7-2600K)

Is this what you mean by the double-refresh?
Comment 5 Mike Fleetwood 2013-04-07 12:35:11 UTC
Hi Curtis,

I've uploaded this screencast:
    GParted bug 696149 demonstration video #1
    http://www.youtube.com/watch?v=__CeSQ7MCiw
(It's only about 840K but I though Youtube was better than GNOME
Bugzilla).

Test case in the video:
    GParted already open
    Just press Ctrl-R to refresh devices

Details of what is happening and what I see in the video:

00:00:03 - 00:00:05  Alt-Tab to GParted window
00:00:11             Ctrl-R (Device Refresh) pressed
00:00:11 - 00:00:13  GParted scanning devices
00:00:13             Slowed down screen redraw starts

                     Device     Graphical  Partition
                     selector   disk       details
                     combobox          
                     ---------  ---------  ---------
00:00:13 - 00:00:16  BLANK (X)  BLANK (X)  BLANK (X)
00:00:16 - 00:00:18  BLANK (X)  drawn      drawn
00:00:18 - 00:00:21  drawn      BLANK (X)  BLANK (X)
00:00:21             drawn      drawn      drawn

00:00:21             Slowed down screen redraw completed
00:00:28 - 00:00:29  Alt-Tab away from GParted window

When this runs at full speed, without the sleeps, having the graphical
disk and partition details go blank and back do drawn causes a flashing
effect during the redraw.  This is what I am seeing as flicker.  Before
bug 667365 "Free space should be selected by default" was applied the
graphical disk and partition details were never blanked during the
redraw, so no flickering occurred.

Additionally, as you said, the device selector combobox is also set to
blank and redrawn.  Watching closely I can now see this flashing during
the redraw with GParted 0.14.1 too, but as it was only a single piece of
text I never noticed it before.  This blanking and redraw of the
combobox also seems ties in with two executions of Refresh_Visual().

Thanks,
Mike
Comment 6 Mike Fleetwood 2013-04-07 13:48:59 UTC
Another screencast:
    GParted bug 696149 demonstration video #2
    http://www.youtube.com/watch?v=VfS-cR-MJn0

Test case in the video:
    Same as video #1 in comment #5, but using GParted 0.14.1
    GParted already open
    Just press Ctrl-R to refresh devices

Details of what is happening and what I see in the video:

00:00:03 - 00:00:04  Alt-Tab to GParted window
00:00:06             Ctrl-R (Device Refresh) pressed
00:00:06 - 00:00:09  GParted scanning devices
00:00:09             Slowed down screen redraw starts

                     Device     Graphical  Partition
                     selector   disk       details
                     combobox          
                     ---------  ---------  ---------
00:00:11 - 00:00:16  BLANK (X)  drawn      drawn
00:00:16 - 00:00:19  drawn      drawn      drawn

00:00:19             Slowed down screen redraw completes
00:00:24             Alt-Tab away from GParted window

Hopefully comparing the videos and timeline details it will be clear
what the difference is in the redraw that is causing the flicker.
Comment 7 Curtis Gedak 2013-04-07 17:05:35 UTC
Thank you Mike for the videos for both GParted 0.15.0 and 0.14.1.  These
have certainly helped me with understanding the problem.

I think the core of the problem arises in the
Win_GParted::Refresh_Visual() method.

If I recall correctly from my early graphics teachings, the steps when
drawing graphics on a display are:

1)  Turn off screen updates
2)  Draw the graphics in screen memory
3)  Turn on screen updates

If the screen updates are not turned off, then the screen will display
the graphics at various stages as these are drawn.  This is what I
think we are seeing.

Refresh_Visual() contains the following graphic updates:

 - The application of each operation (e.g., create, delete, resize)

 - Loading the partitions into the visual disk

 - loading the partitions into the tree view
                                                    ---+
 - Setting the selected partition in the visual disk   |
                                                       +---  NEW in 0.15.0
 - Setting the selected partition in the tree view     |
                                                    ---+

The fact that you can see an extra flicker or refresh with 0.15.0, and
that even 0.14.1 had some flicker/refresh problems fits with this
theory.

At the moment I am short on time to look into the graphic calls to
effectively hide the drawing operations until completed, and then
showing the final result.  If you would like to look into this, then
please feel free.  Otherwise it might be a while before I get a chance
to dig deeper into this problem.
Comment 8 Mike Fleetwood 2013-04-09 17:02:45 UTC
Thanks Curtis, I was most keen for you to see the flashing during
redraw,  hence the videos above with slowed down redraw.  I continue to
be flummoxed as to why this flashing doesn't occur on your machine, but
at least you've seen the fault I am seeing.

I'm continuing to slowly investigate this bug.
Comment 9 Mike Fleetwood 2014-08-28 21:23:43 UTC
Created attachment 284758 [details] [review]
Fix flashing of devices combobox, method 1 (draft 1)
Comment 10 Mike Fleetwood 2014-08-28 21:25:33 UTC
Created attachment 284759 [details] [review]
Fix flashing of devices combobox, method 2 (draft 1)

Hi Curtis,

Here are 2 different methods for fixing the flashing redraw of the
devices combobox.

Methods:
1) Temporarily block the change signal while reconstructing the model
   (list of devices behind the combobox);
2) Create a new model and assign it to the combobox replacing the
   original.

Lots more details in the draft commit messages attached.


As I don't really know which method is better I have posted a question
into the gtk-app-devel-list (Writing Apps with GTK+).

Preferred method for updating the model behind a combobox?
https://mail.gnome.org/archives/gtk-app-devel-list/2014-August/msg00043.html

If you have a preference which method is used I can use that one.


Also note that fixing the redraw of the combobox doesn't fully fix all
flashing redraw issues, but it does make it a log better.  Still looking
into the other bits.

Thanks,
Mike
Comment 11 Curtis Gedak 2014-09-02 17:25:43 UTC
Hi Mike,

From a quick search through the code, it looks like we have both deprecated OptionMenu(s) and the newer ComboBox(es).

Do the OptionMenu GUI items flash when redisplayed on your screen?

If not then perhaps it might be worth digging a little deeper into OptionMenu(s) to discover any key differences.

Regarding the two method patches to address the flashing, both seem to be about the same amount of code and complexity.

When I first got started in graphics I recall using two separate sections of memory.  The active screen would be displayed from one section while drawing operations were used on the second section.  Then when the second section was completed I would switch the display to the second section.  This eliminated several graphic display problems, and is similar to what you implemented in method 2.  I believe this method is called page-flipping.

I'm not saying that method 2 is better, just that I am familiar with this method.  The only downside I can think of is that this requires allocating new memory on each redraw, and each memory allocation has the potential to create a memory leak.

Method 1 makes sense to me too.  Simply tell the signal handler it ignore signals until after the changes are completed, then re-enabling the signals.

Curtis
Comment 12 Mike Fleetwood 2014-09-02 21:43:05 UTC
Hi Curtis,

(In reply to comment #11)
> From a quick search through the code, it looks like we have both
> deprecated OptionMenu(s) and the newer ComboBox(es).
> 
> Do the OptionMenu GUI items flash when redisplayed on your screen?

Assuming the OptionMenu item you are referring to is the sub-menu
GParted > Device selector, then on a quick test it doesn't appear to
flash.


> If not then perhaps it might be worth digging a little deeper into
> OptionMenu(s) to discover any key differences.

One reason is that the sub-menu item GParted > Devices isn't on display
while GParted is refreshing the disk information [Ctrl-R] and destroying
the list of displayed devices and re-creating it.  It is only on display
while choosing which of the known disks to display.  Where as the device
combobox selector is on display with an active selection all the time,
including while refreshing the disk information and destroying and
recreating the list of devices.


> Regarding the two method patches to address the flashing, both seem to
> be about the same amount of code and complexity.
> 
> When I first got started in graphics I recall using two separate
> sections of memory.  The active screen would be displayed from one
> section while drawing operations were used on the second section.
> Then when the second section was completed I would switch the display
> to the second section.  This eliminated several graphic display
> problems, and is similar to what you implemented in method 2.  I
> believe this method is called page-flipping.
> 
> I'm not saying that method 2 is better, just that I am familiar with
> this method.  The only downside I can think of is that this requires
> allocating new memory on each redraw, and each memory allocation has
> the potential to create a memory leak.
> 
> Method 1 makes sense to me too.  Simply tell the signal handler it
> ignore signals until after the changes are completed, then re-enabling
> the signals.

I think method 1 requires nearly as much memory allocation as method 2.
Method 1 clears the Gtk::ListStore, freeing existing rows, and then
creates a new row for each device appended to the ListStore.  Method 2
creates a new ListStore object appending a new row object for each
device as in method 1.  Then it replaces the old ListStore behind the
combobox with the new one.

Not sure if I'm reading what you intended correctly
> ...this requires allocating new memory on each redraw, ...
but this memory allocation is occurring when GParted is refreshing the
list disks, [Ctrl-R], and is replacing the model behind the UI, not
when just redrawing the UI.

> ..., and each memory allocation has the potential to create a memory
> leak.
Agreed.  I ran both methods under valgrind.  Method 2 generates some
more potential leeks.  I assume valgrind reports potential leeks because
it doesn't understand Glib::RefPtr<Gtk::ListStore> and other Gtkmm
objects properly.  I also assume that in method 2 the original
Gtk::ListStore object, and everything it points to, is freed when
nothing refers to it when it is replaced from the model behind the
combobox using .set_model().  The only difference is that in method 1
the same ListStore object lasts for the life of the program where as in
method 2 a new ListStore object is created every refresh.  The rows are
allocated and freed every refresh in both methods.

I would prefer method 2 as well is if wasn't for not being 100%
confident in the above description.

Mike
Comment 13 Mike Fleetwood 2014-10-02 13:46:54 UTC
Created attachment 287587 [details] [review]
Fix flashing redraw (v1)

Hi Curtis,

Here's the fixes for this.  I decided to go with method 1 to fix the
device list ComboBox refresh issue in the end.  Also note that the
second fix is only a work around, but it is as good as I can manage to
workout.  It reduces the flicker so I can't see it, but it does still
redraw the partition list without any selection and draw the selection
of the largest unallocated partition afterwards.

Also as noted in the commit messages, Ubunbu 12.04 LTS doesn't exibit
the flashing redraw, every other version of Ubuntu and every other
distro does show the issue.

Thanks,
Mike
Comment 14 Mike Fleetwood 2014-10-03 07:46:34 UTC
Created attachment 287640 [details] [review]
Fix flashing redraw (v2)

Hi Curtis,

(In reply to comment #13)
> Also as noted in the commit messages, Ubunbu 12.04 LTS doesn't exibit
> the flashing redraw, every other version of Ubuntu and every other
> distro does show the issue.
Which probably explains why you didn't see it.

Here's version 2 of the patchset.

Updates to second patch:
* Adds processing of Gtk events after setting selection.  Not required
  as they get done by the Gtk main loop fractions of a second later, but
  done for consistency with the other code.  See:
    grep Gtk::Main::iteration *.cc
* Add code comments about what is being done and why.

Thanks,
Mike
Comment 15 Curtis Gedak 2014-10-07 17:20:34 UTC
Hi Mike,

The fix flashing redraw (v2) patch set in comment #14 looks good to me.

I tested GParted both with and without the patch set on my physical computer with Kubuntu 12.04 and Debian Testing (8 - Jessie to be).  To my eyes I do not see the flashing, and the gparted appears to run exactly the same.

Since you do experience the flashing on redraw then there must be others that see it too.  As such I am ready to commit this patch set.

Do you have any other changes in mind for this patch set, or shall I commit it to the repository?

Curtis
Comment 16 Mike Fleetwood 2014-10-07 19:31:38 UTC
Hi Curtis,

No further changes planned.  Commit away.

Thanks,
Mike
Comment 17 Curtis Gedak 2014-10-08 00:52:58 UTC
The patch set in comment #14 has been committed to the GNOME repository for inclusion in the next release of GParted.

The relevant git commits can be viewed at the following links:

Prevent flashing redraw of the devices combobox (#696149)
https://git.gnome.org/browse/gparted/commit/?id=52ee26f971a097c0f73b49cfd08d79198d376fd3

Reduce flashing redraw from automatic partition selection (#696149)
https://git.gnome.org/browse/gparted/commit/?id=0fb8cce6992f238b78272b603b69e070cfd46a00
Comment 18 Curtis Gedak 2014-10-20 17:11:37 UTC
This enhancement was included in the GParted 0.20.0 release on October 20, 2014.