Bug 46258 - [EFL] Add support for scaling the contents
Summary: [EFL] Add support for scaling the contents
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-22 05:40 PDT by Alex Bredariol Grilo
Modified: 2011-09-22 01:29 PDT (History)
14 users (show)

See Also:


Attachments
Patch (54.03 KB, patch)
2010-09-22 05:51 PDT, Alex Bredariol Grilo
no flags Details | Formatted Diff | Diff
Patch (51.30 KB, patch)
2010-09-22 07:05 PDT, Alex Bredariol Grilo
no flags Details | Formatted Diff | Diff
Patch (49.13 KB, patch)
2010-12-06 08:47 PST, Alex Bredariol Grilo
tonikitoo: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Bredariol Grilo 2010-09-22 05:40:30 PDT
[EFL] Add support for scaling the contents
Comment 1 Alex Bredariol Grilo 2010-09-22 05:51:51 PDT
Created attachment 68361 [details]
Patch
Comment 2 Alex Bredariol Grilo 2010-09-22 07:05:25 PDT
Created attachment 68367 [details]
Patch
Comment 3 Kenneth Rohde Christiansen 2010-09-22 10:05:47 PDT
Hyatt, there are some ScrollView changes here that it would be nice if you could have a look at.
Comment 4 Kenneth Rohde Christiansen 2010-09-22 10:23:45 PDT
Comment on attachment 68367 [details]
Patch

Why do you guys need all these changes? Can't you scale in the painting system instead? That works pretty well for Qt and we do not require changes like the above.
Comment 5 Alex Bredariol Grilo 2010-09-22 10:44:24 PDT
(In reply to comment #4)

Kenneth, all these changes were made because all scrollbars and innerframes are scaled too. We also apply scaling in the painting system as you said, to get the contents scaled
Comment 6 Kenneth Rohde Christiansen 2010-09-22 10:52:41 PDT
You don't want those to be scaled or?
Comment 7 Alex Bredariol Grilo 2010-09-22 11:14:43 PDT
(In reply to comment #6)
> You don't want those to be scaled or?

We want them to be scaled. The patch includes this feature.
Comment 8 Kenneth Rohde Christiansen 2010-11-17 13:13:15 PST
Comment on attachment 68367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68367&action=review

> WebCore/platform/ScrollView.cpp:148
> -        updateScrollbars(scrollOffset());
> +        updateScrollbars(m_scrollOffset);

scrollOffset should be inline anyway, I do not really like this change

> WebCore/platform/ScrollView.cpp:223
> -    return IntRect(IntPoint(m_scrollOffset.width(), m_scrollOffset.height()),
> -                   IntSize(max(0, width() - (verticalScrollbar() && !includeScrollbars ? verticalScrollbar()->width() : 0)), 
> -                           max(0, height() - (horizontalScrollbar() && !includeScrollbars ? horizontalScrollbar()->height() : 0))));
> +
> +    int w = width() - (verticalScrollbar() && !includeScrollbars ? verticalScrollbar()->width() : 0);
> +    int h = height() - (horizontalScrollbar() && !includeScrollbars ? horizontalScrollbar()->height() : 0);
> +
> +    if (useScale) {
> +        float scale = (this == root()) ? scaleFactor() : 1.0f;
> +        return IntRect(static_cast<int>(ceil(m_scrollOffset.width() / scale)),
> +                       static_cast<int>(ceil(m_scrollOffset.height() / scale)),
> +                       max(0, static_cast<int>(ceil(w / scale))),
> +                       max(0, static_cast<int>(ceil(h / scale))));
> +    } else {
> +        return IntRect(m_scrollOffset.width(), m_scrollOffset.height(),
> +                       max(0, w), max(0, h));
> +    }

Hyatt needs to look at this. This probably also wont work for webkit2. I think you need something like the setActualVisibleContentRect that we introduced a little while ago. I will have some patches soon that makes that actually work 100%.
Comment 9 Andreas Kling 2010-11-17 13:33:52 PST
Comment on attachment 68367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68367&action=review

Whoa, this one is all over the place.
Please break this up into smaller patches, preferably separating the EFL-specific changes from the WebCore changes.

> WebCore/platform/ScrollView.cpp:870
> -    context->clip(visibleContentRect());
> +    IntRect visibleRect = visibleContentRect();
> +    context->clip(visibleRect);

What's going on here?

> WebCore/rendering/RenderBoxModelObject.cpp:114
> -    bool contextIsScaled = !currentTransform.isIdentityOrTranslationOrFlipped();
> +    bool contextIsScaled = false;

Please explain this change.
Comment 10 Alex Bredariol Grilo 2010-11-19 06:23:37 PST
(In reply to comment #9)
> (From update of attachment 68367 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68367&action=review
> 
> > WebCore/platform/ScrollView.cpp:870
> > -    context->clip(visibleContentRect());
> > +    IntRect visibleRect = visibleContentRect();
> > +    context->clip(visibleRect);
> 
> What's going on here?

It remains of some previous multiplication that was removed later.
Sorry for that, it will be removed in the updated patch.

> > WebCore/rendering/RenderBoxModelObject.cpp:114
> > -    bool contextIsScaled = !currentTransform.isIdentityOrTranslationOrFlipped();
> > +    bool contextIsScaled = false;
> 
> Please explain this change.

Sorry for that too. This remains of an old implementation. It will
also be removed in the updated patch.
Comment 11 Alex Bredariol Grilo 2010-12-06 08:47:35 PST
Created attachment 75701 [details]
Patch
Comment 12 Alex Bredariol Grilo 2010-12-06 09:11:08 PST
This patch corrects the problems pointed by Kling and the first problem pointed by Kenneth. We are still working on the other suggestion made by Kenneth.

Hyatt, do you have any other suggestion about the patch?
Comment 13 Dave Hyatt 2010-12-06 09:20:05 PST
Isn't this entire patch unnecessary?  Beth already added fixed scaling using transforms.  Is there any reason why that doesn't work for you?

See the page scale stuff.

Frame.h: pageScaleFactor()

etc.

I think that does what you want... perfectly applies a scale without doing any relayout.
Comment 14 Antonio Gomes 2010-12-06 09:36:21 PST
Comment on attachment 75701 [details]
Patch

it changes much of cross platform code, and comes without a single test. that wont get in like this I think.
Comment 15 WebKit Review Bot 2010-12-07 21:33:57 PST
Attachment 75701 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/6725106
Comment 16 Ryuan Choi 2011-09-22 01:29:43 PDT
Close bug because WebCore already have this feature as another way.