Summary: | [EFL] viewport must be same with the size of webview | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hunseop Jeong <hs85.jeong> | ||||||||||
Component: | WebKit EFL | Assignee: | 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
Hunseop Jeong
2013-11-28 04:33:28 PST
Created attachment 217988 [details]
Patch
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 ? Created attachment 218028 [details]
Patch
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*. (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. I report the new one, changed the title name. (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. Changed the title name Reopened, I don't know how to change the title name. Sorry. *** Bug 124998 has been marked as a duplicate of this bug. *** Created attachment 218035 [details]
Patch
(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 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," (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. Created attachment 218036 [details]
Patch
Comment on attachment 218036 [details]
Patch
It would be nicer if Ryuan or Hyowon take a final look before landing.
Comment on attachment 218036 [details] Patch Clearing flags on attachment: 218036 Committed r159865: <http://trac.webkit.org/changeset/159865> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 125037 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. |