Refactor texture related functions arguments
Reported by
apisa...@yandex-team.ru,
Oct 4 2017
|
|||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36 Steps to reproduce the problem: In some functions, related to texture manipulation: 1) https://cs.chromium.org/chromium/src/third_party/WebKit/public/platform/WebMediaPlayer.h?l=218 2) https://cs.chromium.org/chromium/src/third_party/WebKit/public/platform/WebMediaPlayer.h?l=235 3) https://cs.chromium.org/chromium/src/third_party/WebKit/public/platform/WebMediaPlayer.h?l=255 the same group of arguments used. And this functions passing it further to https://cs.chromium.org/chromium/src/media/renderers/skcanvas_video_renderer.cc?l=866 and then to https://cs.chromium.org/chromium/src/gpu/command_buffer/client/gles2_interface_autogen.h?l=688 I found there are 3 groups of it, which can be united in a structs: 1) Texture arguments unsigned target, unsigned texture, int level, bool premultiply_alpha, bool flip_y, 2) Texture format unsigned internal_format, unsigned format, unsigned type, 3) Texture offset int xoffset, int yoffset, int zoffset, What is the expected behavior? What went wrong? I think it is good to separate this groups to structures and place somewhere. And use them instead of passing lots of args in random order like we do it now. Did this work before? N/A Chrome version: 61.0.3163.100 Channel: stable OS Version: OS X 10.12.6 Flash Version: Shockwave Flash 27.0 r0 What do you think about it? Can you help me find the best place for it?
,
Oct 5 2017
Untriaged it so that issue gets addressed.
,
Oct 18 2017
+eae for Blink, +chcunningham for media/ Hi apisarev - please reach out to the OWNERs of the code and see if they agree with your idea. If they do, you might want to consider submitting a patch to chromium.
,
Oct 19 2017
Adding liberato@ for media/ texture expertise.
,
Oct 23 2017
,
Oct 27 2017
That would be reasonable. apisarev@, would you be willing to send a CL for this?
,
Oct 29
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by nyerramilli@chromium.org
, Oct 5 2017