Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1536713002:
Apply the Surface Texture transformation matrix during texture copy. (Closed)
|
Created:
5 years ago by Tobias Sargeant Modified:
4 years, 11 months ago CC:
chromium-reviews, darin-cc_chromium.org, feature-media-reviews_chromium.org, jam, mcasas+watch_chromium.org, piman+watch_chromium.org, posciak+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionApply the Surface Texture transformation matrix during texture copy.
Android Surface Textures have an associated UV transform matrix that can
potentially vary from frame to frame. If the texture is copied, it is
useful for the copy to have a consistent orientation, rather than having
to propagate the UV transformation matrix with the copy as well.
BUG=226218
Committed: https://crrev.com/f52b1e6228ee51bb78886d2a17155d3e2acbb0da
Cr-Commit-Position: refs/heads/master@{#368017}
Patch Set 1 #Patch Set 2 : remove inline virtual #Patch Set 3 : Apply stream texture transformation matrix by implemementing CopyTex(Sub)Image #
Total comments: 8
Patch Set 4 : Remove stray logging #
Total comments: 7
Patch Set 5 : Address comments. #
Total comments: 4
Patch Set 6 : rebase #Patch Set 7 : reindent #Patch Set 8 : texture must be flipped during copy, to satisfy webgl conformance tests. #Patch Set 9 : Add comment explaining the need for the extra y flip. #
Messages
Total messages: 55 (21 generated)
Description was changed from ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. Because the Surface Texture is meant to be used with (0,0) meaning the bottom left of the texture, the result of this operation from the perspective of Chromium is a y-flipped texture. BUG=226218 ========== to ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. Because the Surface Texture is meant to be used with (0,0) meaning the bottom left of the texture, the result of this operation from the perspective of Chromium is a y-flipped texture. BUG=226218 ==========
tobiasjs@chromium.org changed reviewers: + reveman@chromium.org, sievers@chromium.org
The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1536713002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1536713002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1536713002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1536713002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1536713002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1536713002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Can we implement CopyTex(Sub)Image instead and handle the transform there?
On 2015/12/21 21:14:55, reveman wrote: > Can we implement CopyTex(Sub)Image instead and handle the transform there? I would be happy to be proven wrong, but I think the problem there lies in exposing the CopyTextureCHROMIUMResourceManager machinery outside of GLES2DecoderImpl.
On 2015/12/21 at 21:21:45, tobiasjs wrote: > On 2015/12/21 21:14:55, reveman wrote: > > Can we implement CopyTex(Sub)Image instead and handle the transform there? > > I would be happy to be proven wrong, but I think the problem there lies in exposing the CopyTextureCHROMIUMResourceManager machinery outside of GLES2DecoderImpl. What part of CopyTextureCHROMIUMResourceManager needs to be exposed outside GLES2DecoderImpl? FYI, this will change how SurfaceTexture backed GLImages are implemented and it adds an explicit copy operation until we have the sync primitives needed to use STs for zero-copy: https://codereview.chromium.org/1419623008
On 2015/12/21 21:46:54, reveman wrote: > On 2015/12/21 at 21:21:45, tobiasjs wrote: > > On 2015/12/21 21:14:55, reveman wrote: > > > Can we implement CopyTex(Sub)Image instead and handle the transform there? > > > > I would be happy to be proven wrong, but I think the problem there lies in > exposing the CopyTextureCHROMIUMResourceManager machinery outside of > GLES2DecoderImpl. > > What part of CopyTextureCHROMIUMResourceManager needs to be exposed outside > GLES2DecoderImpl? > > FYI, this will change how SurfaceTexture backed GLImages are implemented and it > adds an explicit copy operation until we have the sync primitives needed to use > STs for zero-copy: > https://codereview.chromium.org/1419623008 So that CL gives me an idea how this might be done. One issue, though, is that the current StreamTextureAndroid implementation of CopyTexImage expects to just do an UpdateTexImage to fetch the next frame, which will then be drawn with. We want to keep that behaviour, so that in the non sync-ipc case there is no copy done. I guess this could switch behaviours based on the target (update and copy if 2D, update if EXTERNAL_OES). The one case I can think of right now where this might fail is in with non sync-ipc, but canvas or webgl doing a copy on the GPU thread. The stream texture will be bound on the render thread, so the texture id will not be valid on the GPU thread (they aren't in a share group). But we're already in a situation where that doesn't work today, so maybe that's not a huge deal. I'll give that a go tomorrow. It seems like WebView is going to make zero copy use of surface textures difficult because of the 2 separate contexts issue. It would involve shuttling the surface texture binding back and forth between the two contexts, which would require each thread preemptively unbinding the ST after it was used. Does this fit with the sync primitives you're alluding to?
> I'll give that a go tomorrow. PS3 does this. Could you please take a look and tell me what you think?
thanks, I think this is a better solution. a few questions below. https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... File content/common/gpu/stream_texture_android.cc (right): https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:170: if (!owner_stub_ || !surface_texture_.get()) when does this happen and why is it correct to return true in that case? https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:190: LOG(WARNING) << "XXX " << "first copy init"; I assume this is not supposed to land. https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:260: if (target != GL_TEXTURE_EXTERNAL_OES) do we still need this non-TEXTURE_2D code?
https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... File content/common/gpu/stream_texture_android.cc (right): https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:170: if (!owner_stub_ || !surface_texture_.get()) On 2015/12/23 16:05:36, reveman wrote: > when does this happen and why is it correct to return true in that case? This could happen when the command buffer is in the process of being destroyed. Returning true early-outs the texture copy code, which presumably would fail anyway. It also mirrors existing code on line 263 https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:190: LOG(WARNING) << "XXX " << "first copy init"; On 2015/12/23 16:05:36, reveman wrote: > I assume this is not supposed to land. You're right. Sorry I missed it when I went through the diff. I'll remove it in the next PS. https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:260: if (target != GL_TEXTURE_EXTERNAL_OES) On 2015/12/23 16:05:36, reveman wrote: > do we still need this non-TEXTURE_2D code? Do you mean the return false for !2D, !EXTERNAL_OES? I presume it's possible that we could be passed a RECT or CUBE_MAP texture, which the texture copying code above will not handle. The default path may still, and returning false here is the minimal change in behaviour for that case. If you mean the EXTERNAL_OES path that follows, then we do need it because in the non sync-IPC case (and for clank, which doesn't have the context problems that webview has) we still rely on passing this texture directly in a resource, and drawing from it without a copy. Spitzer might change this, but for the moment the current behaviour should stay.
https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... File content/common/gpu/stream_texture_android.cc (right): https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:170: if (!owner_stub_ || !surface_texture_.get()) On 2015/12/23 at 20:06:14, Tobias Sargeant wrote: > On 2015/12/23 16:05:36, reveman wrote: > > when does this happen and why is it correct to return true in that case? > > This could happen when the command buffer is in the process of being destroyed. Returning true early-outs the texture copy code, which presumably would fail anyway. > > It also mirrors existing code on line 263 Ok, maybe fine then but I'm somewhat worried that this might result in us drawing uninitialized content which would be a security issue. returning false would feel better as that would make the decoder code either not draw the texture or initialize it to black. https://codereview.chromium.org/1536713002/diff/60001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:260: if (target != GL_TEXTURE_EXTERNAL_OES) On 2015/12/23 at 20:06:14, Tobias Sargeant wrote: > On 2015/12/23 16:05:36, reveman wrote: > > do we still need this non-TEXTURE_2D code? > > Do you mean the return false for !2D, !EXTERNAL_OES? I presume it's possible that we could be passed a RECT or CUBE_MAP texture, which the texture copying code above will not handle. The default path may still, and returning false here is the minimal change in behaviour for that case. > > If you mean the EXTERNAL_OES path that follows, then we do need it because in the non sync-IPC case (and for clank, which doesn't have the context problems that webview has) we still rely on passing this texture directly in a resource, and drawing from it without a copy. Spitzer might change this, but for the moment the current behaviour should stay. Acknowledged. https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... File content/common/gpu/stream_texture_android.cc (right): https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:253: bool StreamTexture::CopyTexImage(unsigned target) { CopyTexImage unlike CopySubTexImage is supposed to define the storage for texture. See CopyTexImage version in https://codereview.chromium.org/1419623008 where I call TexImage2D. I think we need to do the same here. Btw, can we do the same I'm doing in that patch and call CopyTexSubImage here instead of adding a DoCopyTexture function? https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:291: void StreamTexture::UpdateTransformMatrix() { can we move the contents of this function into DoUpdateTexImage instead? maybe use surface_texture_->GetTransformMatrix directly in the copy code and limit current_matrix_ to track what has been sent to the client as before? that way we don't have to add matrix_modified_.. https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... File content/common/gpu/stream_texture_android.h (right): https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... content/common/gpu/stream_texture_android.h:102: int u_xform_loc_; nit: u_xform_location_ for consistency
I'm still not certain whether returning true is the right thing to do. It seems like there should be a way to say: don't try to do the fallback copy, but also, CopyTexImage has failed - that's a case that's not well supported at the moment. As it stands I've left it as true (no fallback copy) because that's what the existing code does. If you feel that that's the wrong choice, then I'll change it to be false in both cases. https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... File content/common/gpu/stream_texture_android.cc (right): https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:253: bool StreamTexture::CopyTexImage(unsigned target) { On 2015/12/23 23:11:49, reveman wrote: > CopyTexImage unlike CopySubTexImage is supposed to define the storage for > texture. See CopyTexImage version in https://codereview.chromium.org/1419623008 > where I call TexImage2D. I think we need to do the same here. > > Btw, can we do the same I'm doing in that patch and call CopyTexSubImage here > instead of adding a DoCopyTexture function? Done. https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:291: void StreamTexture::UpdateTransformMatrix() { On 2015/12/23 23:11:49, reveman wrote: > can we move the contents of this function into DoUpdateTexImage instead? > > maybe use surface_texture_->GetTransformMatrix directly in the copy code and > limit current_matrix_ to track what has been sent to the client as before? that > way we don't have to add matrix_modified_.. Done. matrix_modfied_ didn't need to be added in any case. I'm still a bit worried that if the listener changes (or is set after the matrix is first updated) then it will never be told what the current matrix is. But given that the existing code doesn't exhibit that problem, I think it's not an issue in practice. https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... File content/common/gpu/stream_texture_android.h (right): https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... content/common/gpu/stream_texture_android.h:102: int u_xform_loc_; On 2015/12/23 23:11:49, reveman wrote: > nit: u_xform_location_ for consistency Done.
Returning false for now is probably wrong but that's fine as it's not new to this patch. Lgtm. https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... File content/common/gpu/stream_texture_android.cc (right): https://codereview.chromium.org/1536713002/diff/80001/content/common/gpu/stre... content/common/gpu/stream_texture_android.cc:291: void StreamTexture::UpdateTransformMatrix() { On 2015/12/24 at 01:28:34, Tobias Sargeant wrote: > On 2015/12/23 23:11:49, reveman wrote: > > can we move the contents of this function into DoUpdateTexImage instead? > > > > maybe use surface_texture_->GetTransformMatrix directly in the copy code and > > limit current_matrix_ to track what has been sent to the client as before? that > > way we don't have to add matrix_modified_.. > > Done. matrix_modfied_ didn't need to be added in any case. I'm still a bit worried that if the listener changes (or is set after the matrix is first updated) then it will never be told what the current matrix is. But given that the existing code doesn't exhibit that problem, I think it's not an issue in practice. Right, seems like we should call UpdateTexImage when adding the stub and listener but that can be done in a follow up as not a new problem.
Thanks, and happy holidays. I'll file a bug to address the final comments in a follow-up CL.
The CQ bit was checked by tobiasjs@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1536713002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1536713002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
tobiasjs@chromium.org changed reviewers: + ccameron@chromium.org, kbr@chromium.org
kbr@ or ccameron@, could you please also take a look? Thanks.
lgtm if this has been tested. https://codereview.chromium.org/1536713002/diff/100001/content/common/gpu/str... File content/common/gpu/stream_texture_android.cc (right): https://codereview.chromium.org/1536713002/diff/100001/content/common/gpu/str... content/common/gpu/stream_texture_android.cc:102: glDeleteBuffersARB(1, &vertex_buffer_); It's confusing that we standardized on the ARB naming of this function in the GL bindings, especially since on Android there are no ARB functions. https://codereview.chromium.org/1536713002/diff/100001/content/common/gpu/str... content/common/gpu/stream_texture_android.cc:306: const char kVertexShader[] = STRINGIZE( The indentation looks incorrect for these blocks.
Thanks Ken. This is tested by virtue of enabling a parallel CL that I'm working on. I'll look into whether it is possible to create a unit test for it as well, however. https://codereview.chromium.org/1536713002/diff/100001/content/common/gpu/str... File content/common/gpu/stream_texture_android.cc (right): https://codereview.chromium.org/1536713002/diff/100001/content/common/gpu/str... content/common/gpu/stream_texture_android.cc:102: glDeleteBuffersARB(1, &vertex_buffer_); On 2015/12/28 23:35:47, Ken Russell wrote: > It's confusing that we standardized on the ARB naming of this function in the GL > bindings, especially since on Android there are no ARB functions. Acknowledged. https://codereview.chromium.org/1536713002/diff/100001/content/common/gpu/str... content/common/gpu/stream_texture_android.cc:306: const char kVertexShader[] = STRINGIZE( On 2015/12/28 23:35:47, Ken Russell wrote: > The indentation looks incorrect for these blocks. Done.
The CQ bit was checked by tobiasjs@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org, reveman@chromium.org Link to the patchset: https://codereview.chromium.org/1536713002/#ps140001 (title: "reindent")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1536713002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1536713002/140001
Message was sent while issue was closed.
Description was changed from ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. Because the Surface Texture is meant to be used with (0,0) meaning the bottom left of the texture, the result of this operation from the perspective of Chromium is a y-flipped texture. BUG=226218 ========== to ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. Because the Surface Texture is meant to be used with (0,0) meaning the bottom left of the texture, the result of this operation from the perspective of Chromium is a y-flipped texture. BUG=226218 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. Because the Surface Texture is meant to be used with (0,0) meaning the bottom left of the texture, the result of this operation from the perspective of Chromium is a y-flipped texture. BUG=226218 ========== to ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. Because the Surface Texture is meant to be used with (0,0) meaning the bottom left of the texture, the result of this operation from the perspective of Chromium is a y-flipped texture. BUG=226218 Committed: https://crrev.com/907d1b74f2f3b50d77577c8d83c0cd2472400b3f Cr-Commit-Position: refs/heads/master@{#367714} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/907d1b74f2f3b50d77577c8d83c0cd2472400b3f Cr-Commit-Position: refs/heads/master@{#367714}
Message was sent while issue was closed.
A revert of this CL (patchset #7 id:140001) has been created in https://codereview.chromium.org/1569543002/ by lukasza@chromium.org. The reason for reverting is: The CL was/is the main suspect for causing crbug.com/574811..
Message was sent while issue was closed.
Description was changed from ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. Because the Surface Texture is meant to be used with (0,0) meaning the bottom left of the texture, the result of this operation from the perspective of Chromium is a y-flipped texture. BUG=226218 Committed: https://crrev.com/907d1b74f2f3b50d77577c8d83c0cd2472400b3f Cr-Commit-Position: refs/heads/master@{#367714} ========== to ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. BUG=226218 ==========
Hi Ken, Could you please take a look at this before I resubmit? Unfortunately I don't think I can run the full conformance test suite via a trybot, and I'm OOO, so running the full suite locally is tricky. I've checked the specific cases that I broke, however. Thanks.
On 2016/01/07 00:11:42, Tobias Sargeant wrote: > Hi Ken, > > Could you please take a look at this before I resubmit? Unfortunately I don't > think I can run the full conformance test suite via a trybot, and I'm OOO, so > running the full suite locally is tricky. I've checked the specific cases that I > broke, however. > > Thanks. It looks OK, but what code paths is this used for? Is this going to break the display of hardware-accelerated video on web pages at the expense of passing these WebGL conformance tests? Or was stuff like HW-accelerated video broken with your earlier patch, too (and just not noticed)?
On 2016/01/07 01:10:17, Ken Russell wrote: > It looks OK, but what code paths is this used for? Is this going to break the > display of hardware-accelerated video on web pages at the expense of passing > these WebGL conformance tests? Or was stuff like HW-accelerated video broken > with your earlier patch, too (and just not noticed)? Prior to this patch the surface texture matrix is ignored. In most cases this was actually ok because the android SurfaceTexture is actually in the orientation that chromium expects, and so the identity matrix is correct. Initially I intended to apply the ST matrix here (which results in a y flip) and then mark the video frame y-flipped. Unfortunately I didn't realise that there were other sites where video textures are copied, and those expect to get back an unflipped copy. Also this needs to be consistent with the fallback case. Nothing was broken before, and I just need to update the companion cl to reflect the changed orientation.
On 2016/01/07 02:50:02, Tobias Sargeant wrote: > On 2016/01/07 01:10:17, Ken Russell wrote: > > It looks OK, but what code paths is this used for? Is this going to break the > > display of hardware-accelerated video on web pages at the expense of passing > > these WebGL conformance tests? Or was stuff like HW-accelerated video broken > > with your earlier patch, too (and just not noticed)? > > Prior to this patch the surface texture matrix is ignored. In most cases this > was actually ok because the android SurfaceTexture is actually in the > orientation that chromium expects, and so the identity matrix is correct. > > Initially I intended to apply the ST matrix here (which results in a y flip) and > then mark the video frame y-flipped. Unfortunately I didn't realise that there > were other sites where video textures are copied, and those expect to get back > an unflipped copy. Also this needs to be consistent with the fallback case. > > Nothing was broken before, and I just need to update the companion cl to reflect > the changed orientation. OK, thanks for the explanation -- lgtm if it passes the tests.
The CQ bit was checked by tobiasjs@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from reveman@chromium.org Link to the patchset: https://codereview.chromium.org/1536713002/#ps180001 (title: "Add comment explaining the need for the extra y flip.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1536713002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1536713002/180001
Message was sent while issue was closed.
Description was changed from ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. BUG=226218 ========== to ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. BUG=226218 ==========
Message was sent while issue was closed.
Committed patchset #9 (id:180001)
Message was sent while issue was closed.
Description was changed from ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. BUG=226218 ========== to ========== Apply the Surface Texture transformation matrix during texture copy. Android Surface Textures have an associated UV transform matrix that can potentially vary from frame to frame. If the texture is copied, it is useful for the copy to have a consistent orientation, rather than having to propagate the UV transformation matrix with the copy as well. BUG=226218 Committed: https://crrev.com/f52b1e6228ee51bb78886d2a17155d3e2acbb0da Cr-Commit-Position: refs/heads/master@{#368017} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/f52b1e6228ee51bb78886d2a17155d3e2acbb0da Cr-Commit-Position: refs/heads/master@{#368017} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
