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 668610 - Should show calibration success
Should show calibration success
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Wacom
git master
Other Linux
: Normal normal
: ---
Assigned To: Peter Hutterer
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-01-24 20:25 UTC by Bastien Nocera
Modified: 2012-07-16 16:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (4.16 KB, patch)
2012-07-05 13:45 UTC, Olivier Fourdan
none Details | Review
Updated patch (5.29 KB, patch)
2012-07-13 08:42 UTC, Olivier Fourdan
none Details | Review
Updated patch (5.29 KB, patch)
2012-07-13 08:49 UTC, Olivier Fourdan
needs-work Details | Review
Screenshot of a successful calibration (337.00 KB, image/png)
2012-07-13 10:55 UTC, Olivier Fourdan
  Details
Updated patch (5.68 KB, patch)
2012-07-16 09:56 UTC, Olivier Fourdan
needs-work Details | Review
Screenshot of a successful calibration (123.71 KB, image/png)
2012-07-16 09:56 UTC, Olivier Fourdan
  Details
Updated patch (5.73 KB, patch)
2012-07-16 11:16 UTC, Olivier Fourdan
none Details | Review
Screenshot of a successful calibration (299.48 KB, image/png)
2012-07-16 15:41 UTC, Olivier Fourdan
  Details
Updated patch (6.30 KB, patch)
2012-07-16 15:42 UTC, Olivier Fourdan
none Details | Review
Updated patch (6.31 KB, patch)
2012-07-16 15:51 UTC, Olivier Fourdan
none Details | Review
Updated patch (6.31 KB, patch)
2012-07-16 15:56 UTC, Olivier Fourdan
committed Details | Review

Description Bastien Nocera 2012-01-24 20:25:12 UTC
> - calibration seems to complete fine, but doesn't give any indication
> whether it actually succeeded or did anything at all. Maybe we should
> put a "Last calibration: " label next to the calibrate button (and maybe
> change it to "Just now" and make it green right after the calibration?)
Comment 1 Jakub Steiner 2012-01-26 00:52:59 UTC
I would prefer to avoid using the panel for this. Let's just show a checkmark at the end of the process. I've mocked up something fancy, but a static symbol will do.

http://jimmac.fedorapeople.org/gnome3/tablet-calibration-ok.webm
Comment 2 Olivier Fourdan 2012-07-05 13:45:40 UTC
Created attachment 218082 [details] [review]
Proposed patch

Proposed patch to show a large green tick for 0.5 second when calibration is successful.

Screencast here: http://people.redhat.com/ofourdan/gnome3/show-calibration-success-bug-669610.webm
Comment 3 Olivier Fourdan 2012-07-13 08:42:08 UTC
Created attachment 218687 [details] [review]
Updated patch

Updated patch:

 - Do not use a new gtk_main() loop but gtk_main_iteration(); as gtk+ itself does (in GtkWidget's gtk_widget_show_now())
 - While showing the final success tick, Don't handle signals that may cause reentrency on the finish() routine
 - Print the help lines and points when showing the final tick, to mimic the mock-up
 - Increase the showing time to 750ms (not really sure what correct value would be, 0.5s seems too short, 1sec definitely too long...)
Comment 4 Olivier Fourdan 2012-07-13 08:49:55 UTC
Created attachment 218688 [details] [review]
Updated patch
Comment 5 Bastien Nocera 2012-07-13 09:05:13 UTC
Review of attachment 218688 [details] [review]:

::: panels/wacom/calibrator/gui_gtk.c
@@ +504,3 @@
+	g_timeout_add(END_TIME, (GSourceFunc) draw_success_end_wait_callback , &flag);
+	while (!flag)
+		gtk_main_iteration ();

Do not use gtk_main_iteration().
Comment 6 Olivier Fourdan 2012-07-13 09:24:54 UTC
(In reply to comment #5)
> Review of attachment 218688 [details] [review]:
> Do not use gtk_main_iteration().

I don't know what else to use.

We want the tick to be shown for a short period of time once the whole calibration process is completed, sleeping in the calib_area_finish() function is not suitable and we want gtk events to be processed.

What would you suggest?
Comment 7 Bastien Nocera 2012-07-13 09:41:19 UTC
g_timeout_add() and hide the window then. If you want to be sure that the arrow is shown for a certain time, queue a redraw, and schedule the timeout once you've drawn the arrow.
Comment 8 Olivier Fourdan 2012-07-13 09:51:28 UTC
Yes that would work unless the program calls gtk_main_quit() from the finish callback.

That's what the test-calibrator does, so without that forced iteration loop the tick does not show in the test-calibrator program.

But I guess we don't really care about the test-calibrator anyway so it's safe to avoid that pause.
Comment 9 Bastien Nocera 2012-07-13 09:53:45 UTC
(In reply to comment #8)
> Yes that would work unless the program calls gtk_main_quit() from the finish
> callback.

That needs to be fixed then.

> That's what the test-calibrator does, so without that forced iteration loop the
> tick does not show in the test-calibrator program.
> 
> But I guess we don't really care about the test-calibrator anyway so it's safe
> to avoid that pause.
Comment 10 Olivier Fourdan 2012-07-13 10:55:28 UTC
Created attachment 218699 [details]
Screenshot of a successful calibration
Comment 11 Olivier Fourdan 2012-07-13 11:49:34 UTC
Patch will be reworked to:

 - Use a symbolic icon instead of a tick character (as asked by jimmac) - Icon
   to use is emblem-ok-symbolic from gnome-icon-theme-symbolic master
 - Hide other information while the check-mark is shown (as asked by jimmac)
 - integrate the success notification in the finish callback mechanism so that
   gtk_main_iteration() is avoided
Comment 12 Olivier Fourdan 2012-07-16 09:56:34 UTC
Created attachment 218897 [details] [review]
Updated patch

Updated patch as per comment #11
Comment 13 Olivier Fourdan 2012-07-16 09:56:57 UTC
Created attachment 218898 [details]
Screenshot of a successful calibration
Comment 14 Olivier Fourdan 2012-07-16 10:05:42 UTC
Review of attachment 218897 [details] [review]:

Sorry that patch is broken, the timeout never ends. I shall post a new one shortly.
Comment 15 Bastien Nocera 2012-07-16 10:40:30 UTC
(In reply to comment #8)
> Yes that would work unless the program calls gtk_main_quit() from the finish
> callback.
> 
> That's what the test-calibrator does, so without that forced iteration loop the
> tick does not show in the test-calibrator program.
> 
> But I guess we don't really care about the test-calibrator anyway so it's safe
> to avoid that pause.

We actually care enough about the test-calibrator as it's the only way to see the calibrator UI without the hardware. When I use it, I see the symbolic checkmark being drawn on top of the timeout circle during the whole calibration. Please make sure both the embedded calibrator and the stand-alone one work.
Comment 16 Olivier Fourdan 2012-07-16 11:16:32 UTC
Created attachment 218903 [details] [review]
Updated patch

Patch implementation as per comment #11

(In reply to comment #15)
> We actually care enough about the test-calibrator as it's the only way to see
> the calibrator UI without the hardware. When I use it, I see the symbolic
> checkmark being drawn on top of the timeout circle during the whole
> calibration. Please make sure both the embedded calibrator and the stand-alone
> one work.

The test-calibrator works just fine here with this patch, not sure which version (of the patch) you tried.
Comment 17 Olivier Fourdan 2012-07-16 15:41:04 UTC
Created attachment 218919 [details]
Screenshot of a successful calibration

Updated screenshot
Comment 18 Olivier Fourdan 2012-07-16 15:42:49 UTC
Created attachment 218920 [details] [review]
Updated patch

Updated patch to load the symbolic icon only when needed and use the new gtk_icon API gtk_icon_info_load_symbolic() to render the icon with the appropriate white color which matches the mockup from jimmac.
Comment 19 Olivier Fourdan 2012-07-16 15:51:22 UTC
Created attachment 218921 [details] [review]
Updated patch

Calibration can be successful even if we fail to load the icon to show success.

Update patch, sorry for the noise...
Comment 20 Olivier Fourdan 2012-07-16 15:56:31 UTC
Created attachment 218922 [details] [review]
Updated patch
Comment 21 Bastien Nocera 2012-07-16 16:25:01 UTC
Review of attachment 218922 [details] [review]:

Looks good.