Bug 124965 – [EFL] viewport must be same with the size of webview

• Component: WebKit EFL
• Status: RESOLVED WONTFIX

[Hunseop Jeong]

The Size of the glViewport is set up than the viewSize.

[Hunseop Jeong]

Created attachment 217988 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 217988 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        Fix the size of the glViewport() to draw the correct size.

Different title from this bug.

> Source/WebKit2/ChangeLog:8
> +        The size of the glViewport() is set up than viewSize.

I don't understand what do you want to say in this description. Please re-write description more clear.

> Source/WebKit2/ChangeLog:9
> +        Nevertheless, because the viewport is translated, it looks correctly.

correctly -> correct ?

[Hunseop Jeong]

Created attachment 218028 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 218028 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        [EFL] glViewport() is set up than the viewSize.

I really don't understand what does this title means now. Do you mean that glViewport size is bigger than viewSize ? Anyway, it seems to me that you want to change glViewport size with viewSize, right ?

> Source/WebKit2/ChangeLog:9
> +        Nevertheless, the viewport is translated when being painted, it looks correct.

AFAIK, we should fix something when it is *correct*, not *looks correct*.

[Hunseop Jeong]

(In reply to comment #4)
> (From update of attachment 218028 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=218028&action=review
> 
> > Source/WebKit2/ChangeLog:3
> > +        [EFL] glViewport() is set up than the viewSize.
> 
> I really don't understand what does this title means now. Do you mean that glViewport size is bigger than viewSize ? Anyway, it seems to me that you want to change glViewport size with viewSize, right ?
> 
Yes, right. I mean that the glViewport size is bigger than viewSize.
> > Source/WebKit2/ChangeLog:9
> > +        Nevertheless, the viewport is translated when being painted, it looks correct.
> 
> AFAIK, we should fix something when it is *correct*, not *looks correct*.
Because the glViewport size bigger than viewSize, unnecessary work was occured.

[Hunseop Jeong]

I report the new one, changed the title name.

[Gyuyoung Kim]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 218028 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=218028&action=review
> > 
> > > Source/WebKit2/ChangeLog:3
> > > +        [EFL] glViewport() is set up than the viewSize.
> > 
> > I really don't understand what does this title means now. Do you mean that glViewport size is bigger than viewSize ? Anyway, it seems to me that you want to change glViewport size with viewSize, right ?
> > 
> Yes, right. I mean that the glViewport size is bigger than viewSize.
> > > Source/WebKit2/ChangeLog:9
> > > +        Nevertheless, the viewport is translated when being painted, it looks correct.
> > 
> > AFAIK, we should fix something when it is *correct*, not *looks correct*.
> Because the glViewport size bigger than viewSize, unnecessary work was occured.

When we upload a patch, you should mention *why* this patch should be landed or why we need to land this patch. However, your patch description isn't clear for me. Please fix your changelog more clear. It would be good if you get a review from your co-worker before uploading new patch.

FYI, "unnecessary work was occured." => "unnecessary work occurred" is correct, AFAIK.

[Hunseop Jeong]

Changed the title name

Reopened, I don't know how to change the title name. Sorry.

[Hunseop Jeong]

*** Bug 124998 has been marked as a duplicate of this bug. ***

[Hunseop Jeong]

Created attachment 218035 [details]
Patch

[Hunseop Jeong]

(In reply to comment #7)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (From update of attachment 218028 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=218028&action=review
> > > 
> > > > Source/WebKit2/ChangeLog:3
> > > > +        [EFL] glViewport() is set up than the viewSize.
> > > 
> > > I really don't understand what does this title means now. Do you mean that glViewport size is bigger than viewSize ? Anyway, it seems to me that you want to change glViewport size with viewSize, right ?
> > > 
> > Yes, right. I mean that the glViewport size is bigger than viewSize.
> > > > Source/WebKit2/ChangeLog:9
> > > > +        Nevertheless, the viewport is translated when being painted, it looks correct.
> > > 
> > > AFAIK, we should fix something when it is *correct*, not *looks correct*.
> > Because the glViewport size bigger than viewSize, unnecessary work was occured.
> 
> When we upload a patch, you should mention *why* this patch should be landed or why we need to land this patch. However, your patch description isn't clear for me. Please fix your changelog more clear. It would be good if you get a review from your co-worker before uploading new patch.
> 
> FYI, "unnecessary work was occured." => "unnecessary work occurred" is correct, AFAIK.

Sorry, Gyuyoung Kim. It is my first patch, Because I don't know how to upload patch and write description. I will reviewed by other co-worker when I reported next patch.

[Gyuyoung Kim]

Comment on attachment 218035 [details]
Patch

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

Looks more reasonable than before. However, it looks there are unnecessary *the* in changelog. :(

> Source/WebKit2/ChangeLog:8
> +        Currently, the size of the viewport is set bigger than the size of the webview.

How about using below ?

"Currently, size of viewport is larger than size of webview. Changed the size of viewport with the size of webview because viewport is translated by wrong calculation,"

[Hunseop Jeong]

(In reply to comment #12)
> (From update of attachment 218035 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=218035&action=review
> 
> Looks more reasonable than before. However, it looks there are unnecessary *the* in changelog. :(
> 
> > Source/WebKit2/ChangeLog:8
> > +        Currently, the size of the viewport is set bigger than the size of the webview.
> 
> How about using below ?
> 
> "Currently, size of viewport is larger than size of webview. Changed the size of viewport with the size of webview because viewport is translated by wrong calculation,"

Thanks Gyuyoung Kim.
I will change the changelog, and then upload the patch.

[Hunseop Jeong]

Created attachment 218036 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 218036 [details]
Patch

It would be nicer if Ryuan or Hyowon take a final look before landing.

[WebKit Commit Bot]

Comment on attachment 218036 [details]
Patch

Clearing flags on attachment: 218036

Committed r159865: <http://trac.webkit.org/changeset/159865>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 125037

[Michael Catanzaro]

Closing this bug because the EFL port has been removed from trunk.

If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.