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 778158 - Performance of focus_chain in a common scenario
Performance of focus_chain in a common scenario
Product: gnome-shell
Classification: Core
Component: st
Other Linux
: Normal minor
: ---
Assigned To: gnome-shell-maint
Depends on:
Reported: 2017-02-03 23:18 UTC by Lester Carballo
Modified: 2017-02-09 02:54 UTC
See Also:
GNOME target: ---
GNOME version: ---

widget: Only include visible actors in focus chain (1.32 KB, patch)
2017-02-07 20:17 UTC, Florian Müllner
committed Details | Review

Description Lester Carballo 2017-02-03 23:18:20 UTC
If we want to implement a navigation in our own way, the preferable way will be use the get_focus_chain of St.Widget, to implement our own navigation mechanism.

In a big set of items use the current implementation of this is slow, as st_widget_real_get_focus_chain is implemented in this way: 

It's not filter by (CLUTTER_ACTOR_IS_VISIBLE (child)) and this is a big difference in performance, as we sure have several invisible actors, where we don't want to iterate. The performance issue was introduced in this commit:

There are a way that will be possible, provide another function to compute only the visible actors from the c side? Make it in gjs, it's really slow.
Comment 1 Florian Müllner 2017-02-07 20:17:46 UTC
Created attachment 345144 [details] [review]
widget: Only include visible actors in focus chain

It isn't useful to move the keyboard focus to a hidden actor, so
only include visible actors in the focus chain - this is in fact
the documented behavior of st_widget_get_focus_chain(), so having
the default implementation return all children has always been

(In reply to Lester Carballo from comment #0)
> There are a way that will be possible, provide another function to compute
> only the visible actors from the c side?

I don't think a second function makes sense here - after all, navigate_focus() would still use the first one. We can just adjust get_focus_chain() itself so that it matches the documented behavior ...
Comment 2 Lester Carballo 2017-02-08 02:02:09 UTC
I just thinking this was intentional to be used in some places. Of course the second function it's not needed now... So, thanks. I'm try to get back the Cinnamon gjs code to gnome shell. Most of things are working more faster in gnome shell that in cinnamon, just not the menu:

Thanks again.
Comment 3 Rui Matos 2017-02-08 12:55:49 UTC
Review of attachment 345144 [details] [review]:

Comment 4 Florian Müllner 2017-02-08 13:51:04 UTC
Attachment 345144 [details] pushed as a870a4d - widget: Only include visible actors in focus chain
Comment 5 Lester Carballo 2017-02-09 02:43:09 UTC
What actually could have scene it's get the next focuses actor without a real move of the focus. In some context, you want to cause the impression of a focus change, but not move the focus it's self...

Example: You have the focus in an entry and you don't want to move the focus but, you want to change the selected item at the same time.

st_widget_get_focus_chain, will now work faster in a commons scenarios and will resolved most of cases where the list of actors it's not extra-big... With an extra-big list, i think also the whole shell could notice the impact.
Comment 6 Lester Carballo 2017-02-09 02:54:09 UTC
Something like:

st_widget_next_navigation_focus(StWidget *widget, ClutterActor *from, GtkDirectionType direction)

where the from ClutterActor, not need to have the focus to get the successor in the focus chain.