Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
10817016:
aura: Fix "not-allowed" cursor and hot-points of some other cursors. (Closed)
|
Created:
8 years, 5 months ago by varunjain Modified:
8 years, 4 months ago Reviewers:
sky CC:
chromium-reviews, sadrul, ben+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
Descriptionaura: Fix "not-allowed" cursor and hot-points of some other cursors.
BUG=135254
TEST=manual
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=148498
Patch Set 1 #
Total comments: 2
Patch Set 2 : patch #Messages
Total messages: 12 (0 generated)
https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... File ui/aura/root_window_host_linux.cc (right): https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... ui/aura/root_window_host_linux.cc:325: // src/platforms/assets/cursors/*.cfg files. Is there some better way to communicate this so that it isn't possible for chromeos and chrome to get out of sync?
https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... File ui/aura/root_window_host_linux.cc (right): https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... ui/aura/root_window_host_linux.cc:325: // src/platforms/assets/cursors/*.cfg files. On 2012/07/23 23:00:57, sky wrote: > Is there some better way to communicate this so that it isn't possible for > chromeos and chrome to get out of sync? Actually this is inaccurate. We have new cursor images from Alex with hot points shown in the image. I have updated the comment. I think best way to do this would be to define hot points in cfg files checked in where the cursor images are (ui/resources/aura). If you think thats ok, I can do that in a followup CL. My concerns was if its worth increasing the resources just for the hot points.
Its my understanding that the hot spots are also defined outside of chrome. I was hoping there is a way to have those hotspots defined only in one place. -Scott On Wed, Jul 25, 2012 at 9:38 AM, <varunjain@chromium.org> wrote: > > https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... > File ui/aura/root_window_host_linux.cc (right): > > https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... > ui/aura/root_window_host_linux.cc:325: // > src/platforms/assets/cursors/*.cfg files. > On 2012/07/23 23:00:57, sky wrote: >> >> Is there some better way to communicate this so that it isn't possible > > for >> >> chromeos and chrome to get out of sync? > > > Actually this is inaccurate. We have new cursor images from Alex with > hot points shown in the image. I have updated the comment. I think best > way to do this would be to define hot points in cfg files checked in > where the cursor images are (ui/resources/aura). If you think thats ok, > I can do that in a followup CL. My concerns was if its worth increasing > the resources just for the hot points. > > https://chromiumcodereview.appspot.com/10817016/
On 2012/07/25 16:58:13, sky wrote: > Its my understanding that the hot spots are also defined outside of > chrome. I was hoping there is a way to have those hotspots defined > only in one place. Discussed this with Dan. The cursors defined in chromeos replace the default X cursors. They are still useful because they may show up when chrome crashes or if the pointer enters a non-chrome X client (not sure how or when that could happen). The aura cursors also need to stay because we want to load them from the resource bundle in a high DPI aware manner. So I am not sure how to consolidate the two. > > -Scott > > On Wed, Jul 25, 2012 at 9:38 AM, <mailto:varunjain@chromium.org> wrote: > > > > > https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... > > File ui/aura/root_window_host_linux.cc (right): > > > > > https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... > > ui/aura/root_window_host_linux.cc:325: // > > src/platforms/assets/cursors/*.cfg files. > > On 2012/07/23 23:00:57, sky wrote: > >> > >> Is there some better way to communicate this so that it isn't possible > > > > for > >> > >> chromeos and chrome to get out of sync? > > > > > > Actually this is inaccurate. We have new cursor images from Alex with > > hot points shown in the image. I have updated the comment. I think best > > way to do this would be to define hot points in cfg files checked in > > where the cursor images are (ui/resources/aura). If you think thats ok, > > I can do that in a followup CL. My concerns was if its worth increasing > > the resources just for the hot points. > > > > https://chromiumcodereview.appspot.com/10817016/
Did Dan have any suggestions? Is it possible to use an X API to query? I seem to remember asking Dan about this before and not coming up with anything... -Scott On Wed, Jul 25, 2012 at 10:46 AM, <varunjain@chromium.org> wrote: > On 2012/07/25 16:58:13, sky wrote: >> >> Its my understanding that the hot spots are also defined outside of >> chrome. I was hoping there is a way to have those hotspots defined >> only in one place. > > > Discussed this with Dan. The cursors defined in chromeos replace the default > X > cursors. They are still useful because they may show up when chrome crashes > or > if the pointer enters a non-chrome X client (not sure how or when that could > happen). > The aura cursors also need to stay because we want to load them from the > resource bundle in a high DPI aware manner. So I am not sure how to > consolidate > the two. > > >> -Scott > > >> On Wed, Jul 25, 2012 at 9:38 AM, <mailto:varunjain@chromium.org> wrote: >> > >> > > > > https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... >> >> > File ui/aura/root_window_host_linux.cc (right): >> > >> > > > > https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... >> >> > ui/aura/root_window_host_linux.cc:325: // >> > src/platforms/assets/cursors/*.cfg files. >> > On 2012/07/23 23:00:57, sky wrote: >> >> >> >> Is there some better way to communicate this so that it isn't possible >> > >> > for >> >> >> >> chromeos and chrome to get out of sync? >> > >> > >> > Actually this is inaccurate. We have new cursor images from Alex with >> > hot points shown in the image. I have updated the comment. I think best >> > way to do this would be to define hot points in cfg files checked in >> > where the cursor images are (ui/resources/aura). If you think thats ok, >> > I can do that in a followup CL. My concerns was if its worth increasing >> > the resources just for the hot points. >> > >> > https://chromiumcodereview.appspot.com/10817016/ > > > > https://chromiumcodereview.appspot.com/10817016/
Nope, I didn't have any suggestions here. :-/ On 2012/07/25 21:04:38, sky wrote: > Did Dan have any suggestions? Is it possible to use an X API to query? > I seem to remember asking Dan about this before and not coming up with > anything... > > -Scott > > On Wed, Jul 25, 2012 at 10:46 AM, <mailto:varunjain@chromium.org> wrote: > > On 2012/07/25 16:58:13, sky wrote: > >> > >> Its my understanding that the hot spots are also defined outside of > >> chrome. I was hoping there is a way to have those hotspots defined > >> only in one place. > > > > > > Discussed this with Dan. The cursors defined in chromeos replace the default > > X > > cursors. They are still useful because they may show up when chrome crashes > > or > > if the pointer enters a non-chrome X client (not sure how or when that could > > happen). > > The aura cursors also need to stay because we want to load them from the > > resource bundle in a high DPI aware manner. So I am not sure how to > > consolidate > > the two. > > > > > >> -Scott > > > > > >> On Wed, Jul 25, 2012 at 9:38 AM, <mailto:varunjain@chromium.org> wrote: > >> > > >> > > > > > > > > https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... > >> > >> > File ui/aura/root_window_host_linux.cc (right): > >> > > >> > > > > > > > > https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... > >> > >> > ui/aura/root_window_host_linux.cc:325: // > >> > src/platforms/assets/cursors/*.cfg files. > >> > On 2012/07/23 23:00:57, sky wrote: > >> >> > >> >> Is there some better way to communicate this so that it isn't possible > >> > > >> > for > >> >> > >> >> chromeos and chrome to get out of sync? > >> > > >> > > >> > Actually this is inaccurate. We have new cursor images from Alex with > >> > hot points shown in the image. I have updated the comment. I think best > >> > way to do this would be to define hot points in cfg files checked in > >> > where the cursor images are (ui/resources/aura). If you think thats ok, > >> > I can do that in a followup CL. My concerns was if its worth increasing > >> > the resources just for the hot points. > >> > > >> > https://chromiumcodereview.appspot.com/10817016/ > > > > > > > > https://chromiumcodereview.appspot.com/10817016/
On 2012/07/25 21:04:38, sky wrote: > Did Dan have any suggestions? Is it possible to use an X API to query? > I seem to remember asking Dan about this before and not coming up with > anything... Hi Scott, about your suggestion: I did not find any X query to determine the device scale. In chrome, we do that using code in ui/aura/display_change_observer_x11::NotifyDisplayChange(). If we were to move the high dpi cursors in the chromeos tree, this code will have to be copied there to choose the right resolution image. > > -Scott > > On Wed, Jul 25, 2012 at 10:46 AM, <mailto:varunjain@chromium.org> wrote: > > On 2012/07/25 16:58:13, sky wrote: > >> > >> Its my understanding that the hot spots are also defined outside of > >> chrome. I was hoping there is a way to have those hotspots defined > >> only in one place. > > > > > > Discussed this with Dan. The cursors defined in chromeos replace the default > > X > > cursors. They are still useful because they may show up when chrome crashes > > or > > if the pointer enters a non-chrome X client (not sure how or when that could > > happen). > > The aura cursors also need to stay because we want to load them from the > > resource bundle in a high DPI aware manner. So I am not sure how to > > consolidate > > the two. > > > > > >> -Scott > > > > > >> On Wed, Jul 25, 2012 at 9:38 AM, <mailto:varunjain@chromium.org> wrote: > >> > > >> > > > > > > > > https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... > >> > >> > File ui/aura/root_window_host_linux.cc (right): > >> > > >> > > > > > > > > https://chromiumcodereview.appspot.com/10817016/diff/1/ui/aura/root_window_ho... > >> > >> > ui/aura/root_window_host_linux.cc:325: // > >> > src/platforms/assets/cursors/*.cfg files. > >> > On 2012/07/23 23:00:57, sky wrote: > >> >> > >> >> Is there some better way to communicate this so that it isn't possible > >> > > >> > for > >> >> > >> >> chromeos and chrome to get out of sync? > >> > > >> > > >> > Actually this is inaccurate. We have new cursor images from Alex with > >> > hot points shown in the image. I have updated the comment. I think best > >> > way to do this would be to define hot points in cfg files checked in > >> > where the cursor images are (ui/resources/aura). If you think thats ok, > >> > I can do that in a followup CL. My concerns was if its worth increasing > >> > the resources just for the hot points. > >> > > >> > https://chromiumcodereview.appspot.com/10817016/ > > > > > > > > https://chromiumcodereview.appspot.com/10817016/
*SIGH* LGTM
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/varunjain@chromium.org/10817016/5001
Change committed as 148498
On 2012/07/26 03:09:10, I haz the power (commit-bot) wrote: > Change committed as 148498 drive-by comment. Ideally, hot point should be embedded in png file's custom data section so that the author of icon can specify it and chrome can retrieve it. Just filed crbug.com/141586. We should also do this for 9 patch like data that we're using for text button/omnibox, but that's probably less important than this. |
