From a0cc55037ebbfcabb08eaefaa0e8e92a10b03475 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 19 Sep 2014 11:35:01 +0200 Subject: [PATCH 1/5] webelem: Get rid of functools.wraps/functools.update_wrapper. --- qutebrowser/browser/webelem.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py index 4dde0b5b8..a613409f1 100644 --- a/qutebrowser/browser/webelem.py +++ b/qutebrowser/browser/webelem.py @@ -98,15 +98,15 @@ class WebElementWrapper(collections.abc.MutableMapping): method = getattr(self._elem, name) - @functools.wraps(method) def _wrapper(meth, *args, **kwargs): # pylint: disable=missing-docstring self._check_vanished() return meth(*args, **kwargs) wrapper = functools.partial(_wrapper, method) - functools.update_wrapper(wrapper, method) - + # We used to do functools.update_wrapper here, but for some reason + # when using hints with many links, this accounted for nearly 50% + # of the time when profiling, which is unacceptable. setattr(self, name, wrapper) def __str__(self): From bc884ed51e3488d2a9dd7d64e03b48b0f3b73273 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 19 Sep 2014 12:28:23 +0200 Subject: [PATCH 2/5] webelem: Refuse to wrap a wrapper in WebElementWrapper. --- qutebrowser/browser/webelem.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py index a613409f1..1d2c7d8d7 100644 --- a/qutebrowser/browser/webelem.py +++ b/qutebrowser/browser/webelem.py @@ -70,6 +70,8 @@ class WebElementWrapper(collections.abc.MutableMapping): """A wrapper around QWebElement to make it more intelligent.""" def __init__(self, elem): + if isinstance(elem, self.__class__): + raise TypeError("Trying to wrap a wrapper!") if elem.isNull(): raise IsNullError('{} is a null element!'.format(elem)) self._elem = elem From 2686278b48be4b5535a121d572bac9a443cbec86 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 19 Sep 2014 12:30:38 +0200 Subject: [PATCH 3/5] webelem: Make it possible to use is_visible with unwrapped elements. --- qutebrowser/browser/webelem.py | 127 ++++++++++++++++++++------------- 1 file changed, 78 insertions(+), 49 deletions(-) diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py index 1d2c7d8d7..adc9e8f82 100644 --- a/qutebrowser/browser/webelem.py +++ b/qutebrowser/browser/webelem.py @@ -164,58 +164,11 @@ class WebElementWrapper(collections.abc.MutableMapping): Return: True if the element is visible, False otherwise. """ - self._check_vanished() - # CSS attributes which hide an element - hidden_attributes = { - 'visibility': 'hidden', - 'display': 'none', - } - for k, v in hidden_attributes.items(): - if self._elem.styleProperty(k, QWebElement.ComputedStyle) == v: - return False - geometry = self._elem.geometry() - if not geometry.isValid() and geometry.x() == 0: - # Most likely an invisible link - return False - # First check if the element is visible on screen - elem_rect = self.rect_on_view() - if elem_rect.isValid(): - visible_on_screen = mainframe.geometry().intersects(elem_rect) - else: - # We got an invalid rectangle (width/height 0/0 probably), but this - # can still be a valid link. - visible_on_screen = mainframe.geometry().contains( - elem_rect.topLeft()) - # Then check if it's visible in its frame if it's not in the main - # frame. - elem_frame = self._elem.webFrame() - elem_rect = self._elem.geometry() - framegeom = QRect(elem_frame.geometry()) - if not framegeom.isValid(): - visible_in_frame = False - elif elem_frame.parentFrame() is not None: - framegeom.moveTo(0, 0) - framegeom.translate(elem_frame.scrollPosition()) - if elem_rect.isValid(): - visible_in_frame = framegeom.intersects(elem_rect) - else: - # We got an invalid rectangle (width/height 0/0 probably), but - # this can still be a valid link. - visible_in_frame = framegeom.contains(elem_rect.topLeft()) - else: - visible_in_frame = visible_on_screen - return all([visible_on_screen, visible_in_frame]) + return is_visible(self._elem, mainframe) def rect_on_view(self): """Get the geometry of the element relative to the webview.""" - self._check_vanished() - frame = self._elem.webFrame() - rect = QRect(self._elem.geometry()) - while frame is not None: - rect.translate(frame.geometry().topLeft()) - rect.translate(frame.scrollPosition() * -1) - frame = frame.parentFrame() - return rect + return rect_on_view(self._elem) def is_writable(self): """Check whether an element is writable.""" @@ -381,3 +334,79 @@ def focus_elem(frame): """ elem = frame.findFirstElement(SELECTORS[Group.focus]) return WebElementWrapper(elem) + + +def rect_on_view(elem): + """Get the geometry of the element relative to the webview. + + We need this as a standalone function (as opposed to a WebElementWrapper + method) because we want to run is_visible before wrapping when hinting for + performance reasons. + + Args: + elem: The QWebElement to get the rect for. + """ + if elem.isNull(): + raise IsNullError("Got called on a null element!") + frame = elem.webFrame() + rect = QRect(elem.geometry()) + while frame is not None: + rect.translate(frame.geometry().topLeft()) + rect.translate(frame.scrollPosition() * -1) + frame = frame.parentFrame() + return rect + + +def is_visible(elem, mainframe): + """Check if the given element is visible in the frame. + + We need this as a standalone function (as opposed to a WebElementWrapper + method) because we want to check this before wrapping when hinting for + performance reasons. + + Args: + elem: The QWebElement to check. + mainframe: The QWebFrame in which the element should be visible. + """ + if elem.isNull(): + raise IsNullError("Got called on a null element!") + # CSS attributes which hide an element + hidden_attributes = { + 'visibility': 'hidden', + 'display': 'none', + } + for k, v in hidden_attributes.items(): + if elem.styleProperty(k, QWebElement.ComputedStyle) == v: + return False + geometry = elem.geometry() + if not geometry.isValid() and geometry.x() == 0: + # Most likely an invisible link + return False + # First check if the element is visible on screen + elem_rect = rect_on_view(elem) + if elem_rect.isValid(): + visible_on_screen = mainframe.geometry().intersects(elem_rect) + else: + # We got an invalid rectangle (width/height 0/0 probably), but this + # can still be a valid link. + visible_on_screen = mainframe.geometry().contains( + elem_rect.topLeft()) + # Then check if it's visible in its frame if it's not in the main + # frame. + elem_frame = elem.webFrame() + elem_rect = elem.geometry() + framegeom = QRect(elem_frame.geometry()) + if not framegeom.isValid(): + visible_in_frame = False + elif elem_frame.parentFrame() is not None: + framegeom.moveTo(0, 0) + framegeom.translate(elem_frame.scrollPosition()) + if elem_rect.isValid(): + visible_in_frame = framegeom.intersects(elem_rect) + else: + # We got an invalid rectangle (width/height 0/0 probably), but + # this can still be a valid link. + visible_in_frame = framegeom.contains(elem_rect.topLeft()) + else: + visible_in_frame = visible_on_screen + return all([visible_on_screen, visible_in_frame]) From 658053842e01abeeb3702256048b0790b65101f7 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 19 Sep 2014 12:32:28 +0200 Subject: [PATCH 4/5] hints: Wrap elements after checking is_visible. Wrapping thousands of elements is really slow (>3 seconds for hinting on a reddit page, because we wrap ~2500 elements with 50 methods each), so we try to filter the elements first before wrapping them, as the visible elements will be much less. --- qutebrowser/browser/hints.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 3e050f3c9..9d7705d02 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -507,19 +507,21 @@ class HintManager(QObject): ctx = HintContext() ctx.frames = webelem.get_child_frames(mainframe) for f in ctx.frames: - for e in f.findAllElements(webelem.SELECTORS[group]): - elems.append(webelem.WebElementWrapper(e)) + elems += f.findAllElements(webelem.SELECTORS[group]) + elems = [e for e in elems if webelem.is_visible(e, mainframe)] + # We wrap the elements late for performance reasons, as wrapping 1000s + # of elements (with ~50 methods each) just takes too much time... + elems = [webelem.WebElementWrapper(e) for e in elems] filterfunc = webelem.FILTERS.get(group, lambda e: True) - visible_elems = [e for e in elems if filterfunc(e) and - e.is_visible(mainframe)] - if not visible_elems: + elems = [e for e in elems if filterfunc(e)] + if not elems: raise cmdexc.CommandError("No elements found.") ctx.target = target ctx.baseurl = baseurl ctx.args = args message.instance().set_text(self.HINT_TEXTS[target]) - strings = self._hint_strings(visible_elems) - for e, string in zip(visible_elems, strings): + strings = self._hint_strings(elems) + for e, string in zip(elems, strings): label = self._draw_label(e, string) ctx.elems[string] = ElemTuple(e, label) self._context = ctx From 84cdb30bcb5bc2f1364a9af5f706eab59a1933ab Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 19 Sep 2014 17:39:37 +0200 Subject: [PATCH 5/5] webelem: Avoid unnecessary ::geometry calls --- qutebrowser/browser/webelem.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/qutebrowser/browser/webelem.py b/qutebrowser/browser/webelem.py index adc9e8f82..f0ec7b5f7 100644 --- a/qutebrowser/browser/webelem.py +++ b/qutebrowser/browser/webelem.py @@ -336,7 +336,7 @@ def focus_elem(frame): return WebElementWrapper(elem) -def rect_on_view(elem): +def rect_on_view(elem, elem_geometry=None): """Get the geometry of the element relative to the webview. We need this as a standalone function (as opposed to a WebElementWrapper @@ -345,11 +345,16 @@ def rect_on_view(elem): Args: elem: The QWebElement to get the rect for. + elem_geometry: The geometry of the element, or None. + Calling QWebElement::geometry is rather expensive so we + want to avoid doing it twice. """ if elem.isNull(): raise IsNullError("Got called on a null element!") + if elem_geometry is None: + elem_geometry = elem.geometry() frame = elem.webFrame() - rect = QRect(elem.geometry()) + rect = QRect(elem_geometry) while frame is not None: rect.translate(frame.geometry().topLeft()) rect.translate(frame.scrollPosition() * -1) @@ -378,35 +383,35 @@ def is_visible(elem, mainframe): for k, v in hidden_attributes.items(): if elem.styleProperty(k, QWebElement.ComputedStyle) == v: return False - geometry = elem.geometry() - if not geometry.isValid() and geometry.x() == 0: + elem_geometry = elem.geometry() + if not elem_geometry.isValid() and elem_geometry.x() == 0: # Most likely an invisible link return False # First check if the element is visible on screen - elem_rect = rect_on_view(elem) + elem_rect = rect_on_view(elem, elem_geometry=elem_geometry) + mainframe_geometry = mainframe.geometry() if elem_rect.isValid(): - visible_on_screen = mainframe.geometry().intersects(elem_rect) + visible_on_screen = mainframe_geometry.intersects(elem_rect) else: # We got an invalid rectangle (width/height 0/0 probably), but this # can still be a valid link. - visible_on_screen = mainframe.geometry().contains( + visible_on_screen = mainframe_geometry.contains( elem_rect.topLeft()) # Then check if it's visible in its frame if it's not in the main # frame. elem_frame = elem.webFrame() - elem_rect = elem.geometry() framegeom = QRect(elem_frame.geometry()) if not framegeom.isValid(): visible_in_frame = False elif elem_frame.parentFrame() is not None: framegeom.moveTo(0, 0) framegeom.translate(elem_frame.scrollPosition()) - if elem_rect.isValid(): - visible_in_frame = framegeom.intersects(elem_rect) + if elem_geometry.isValid(): + visible_in_frame = framegeom.intersects(elem_geometry) else: # We got an invalid rectangle (width/height 0/0 probably), but # this can still be a valid link. - visible_in_frame = framegeom.contains(elem_rect.topLeft()) + visible_in_frame = framegeom.contains(elem_geometry.topLeft()) else: visible_in_frame = visible_on_screen return all([visible_on_screen, visible_in_frame])