Bug 124965

Summary: [EFL] viewport must be same with the size of webview
Product: WebKit Reporter: Hunseop Jeong <hs85.jeong>
Component: WebKit EFLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: cdumez, commit-queue, gyuyoung.kim, gyuyoung.kim, hw1008.kim, lucas.de.marchi, mcatanzaro, rakuco
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: Linux   
Bug Depends on: 125007, 125037    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Hunseop Jeong 2013-11-28 04:33:28 PST
The Size of the glViewport is set up than the viewSize.
Comment 1 Hunseop Jeong 2013-11-28 04:39:09 PST
Created attachment 217988 [details]
Patch
Comment 2 Gyuyoung Kim 2013-11-28 15:36:28 PST
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 ?
Comment 3 Hunseop Jeong 2013-11-28 17:15:21 PST
Created attachment 218028 [details]
Patch
Comment 4 Gyuyoung Kim 2013-11-28 19:55:46 PST
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*.
Comment 5 Hunseop Jeong 2013-11-28 21:33:53 PST
(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.
Comment 6 Hunseop Jeong 2013-11-28 22:31:29 PST
I report the new one, changed the title name.
Comment 7 Gyuyoung Kim 2013-11-28 22:34:26 PST
(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.
Comment 8 Hunseop Jeong 2013-11-28 22:44:11 PST
Changed the title name

Reopened, I don't know how to change the title name. Sorry.
Comment 9 Hunseop Jeong 2013-11-28 22:46:13 PST
*** Bug 124998 has been marked as a duplicate of this bug. ***
Comment 10 Hunseop Jeong 2013-11-28 22:59:59 PST
Created attachment 218035 [details]
Patch
Comment 11 Hunseop Jeong 2013-11-28 23:09:48 PST
(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.
Comment 12 Gyuyoung Kim 2013-11-28 23:13:57 PST
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,"
Comment 13 Hunseop Jeong 2013-11-28 23:24:24 PST
(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.
Comment 14 Hunseop Jeong 2013-11-28 23:26:00 PST
Created attachment 218036 [details]
Patch
Comment 15 Gyuyoung Kim 2013-11-28 23:52:33 PST
Comment on attachment 218036 [details]
Patch

It would be nicer if Ryuan or Hyowon take a final look before landing.
Comment 16 WebKit Commit Bot 2013-11-29 00:34:50 PST
Comment on attachment 218036 [details]
Patch

Clearing flags on attachment: 218036

Committed r159865: <http://trac.webkit.org/changeset/159865>
Comment 17 WebKit Commit Bot 2013-11-29 00:34:53 PST
All reviewed patches have been landed.  Closing bug.
Comment 18 WebKit Commit Bot 2013-11-30 19:45:12 PST
Re-opened since this is blocked by bug 125037
Comment 19 Michael Catanzaro 2017-03-11 10:52:40 PST
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.