[Webkit-unassigned] [Bug 35146] Support tiled backing store

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=35146





--- Comment #26 from Simon Hausmann <hausmann at webkit.org>  2010-03-08 02:58:18 PST ---
(From update of attachment 50032)

> +QRectF QGraphicsWebViewPrivate::visibleRect() const
> +{
> +    if (!q->scene())
> +        return QRectF();
> +    QList<QGraphicsView*> views = q->scene()->views();
> +    if (views.size() > 1) {
> +        qDebug() << "QGraphicsWebView is in more than one graphics views, unable to compute the visible rect";
> +        return QRectF();
> +    }
> +    if (views.size() < 1)
> +        return QRectF();
> +
> +    int h = views[0]->horizontalScrollBar()->value();
> +    int v = views[0]->verticalScrollBar()->value();

I suggest to use views.at(0) instead of [] or make the views variable const.
With the current code the compiler will choose the non-const [] operator. As
the returned views is an implicitly shared copy of the list in QGraphicsScene,
the code will end up detaching the entire list.

> +        TiledBackingStoreEnabled,

It would be good to document this enum value in qwebsettings.cpp and mention
the default.


> @@ -724,6 +753,8 @@ void LauncherApplication::handleUserOpti
>               << "[-show-fps]"
>               << "[-r list]"
>               << "[-inspector-url location]"
> +             << "[-tiled-backing-store]"
> +             << "[-resizes-to-contents]"
>               << "URLs";
>          appQuit(0);
>      }
> @@ -746,6 +777,16 @@ void LauncherApplication::handleUserOpti
>          gCacheWebView = true;
>      }
>  
> +    if (args.contains("-tiled-backing-store")) {
> +        requiresGraphicsView("-tiled-backing-store");
> +        gUseTiledBackingStore = true;
> +    }
> +
> +    if (args.contains("-resizes-to-contents")) {
> +        requiresGraphicsView("-resizes-to-contents");
> +        gResizesToContents = true;
> +    }
> +
>      QString arg1("-viewport-update-mode");
>      int modeIndex = args.indexOf(arg1);
>      if (modeIndex != -1) {

Are the command line options really worth it? The launcher code might be
simplier if we support the options only in the menu. Just a thought though :)


> +void WebViewGraphicsBased::setResizesToContents(bool b)
> +{
> +    m_resizesToContents = b;
> +    m_item->setResizesToContents(m_resizesToContents);
> +    if (m_resizesToContents) {
> +        setHorizontalScrollBarPolicy(Qt::ScrollBarAsNeeded);
> +        setVerticalScrollBarPolicy(Qt::ScrollBarAsNeeded);
> +    } else {
> +        setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
> +        setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
> +    }

Hmm, why is this needed? Should setResizesToContents automatically do this? Or
perhaps the docs of the resizesToContents property should explain that this may
also be a good thing to do.

Why are the scrollbars turned _off_ if m_resizesToContents is false? Shouldn't
it be the other way around?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.