Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1921533008:
Add support for OutputSurface for Vulkan on Android. (Closed)
DescriptionAdd support for OutputSurface for Vulkan on Android.
This adds an optional Vulkan Output Surface for Android's
Browser Compositor,using a shared Vulkan Context provider.
BUG=582558
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
Committed: https://crrev.com/2986bd485401025eeef77025dd9697ec36f9d347
Cr-Commit-Position: refs/heads/master@{#392354}
Patch Set 1 #Patch Set 2 : Inherit from OutputSurface instead of OutputSurfaceWithoutParent #
Total comments: 14
Patch Set 3 : use vulkan flag + other review comments. #
Total comments: 6
Patch Set 4 : use static vk cxt provider. #Patch Set 5 : avoid lazyinstance of scopedrefptr #Patch Set 6 : revert back to PS#4 with some modif. #Patch Set 7 : removed extra line breaks. #Patch Set 8 : rebase to ToT. #Patch Set 9 : rebase errors. #
Total comments: 2
Patch Set 10 : fix rebase error. #
Messages
Total messages: 45 (17 generated)
Description was changed from ========== Add support for OutputSurface for Vulkan on Android. BUG=582558 ========== to ========== Add support for OutputSurface for Vulkan on Android. BUG=582558 ==========
sohan.jyoti@samsung.com changed reviewers: + dyen@chromium.org, piman@chromium.org
Please take a look, thanks.
https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... File content/browser/renderer_host/compositor_impl_android.cc (right): https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:230: class VulkanOutputSurface : public cc::OutputSurface { I think this whole class needs to be inside #if defined(ENABLE_VULKAN), because otherwise you won't have an implementation for VulkanSurface. https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:233: const scoped_refptr<cc::VulkanContextProvider>& vulkan_context_provider) nit: scoped_refptr<cc::VulkanContextProvider> (not const scoped_refptr<>&), per style guide. https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:234: : OutputSurface(nullptr, nullptr, vulkan_context_provider, nullptr) {} nit: std::move(vulkan_context_provider) https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:271: std::unique_ptr<gpu::VulkanSurface> surface_; nit: DISALLOW_COPY_AND_ASSIGN() https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:632: DCHECK(context_provider.get()); All this code (from l.560 to here) should only execute when we don't use Vulkan. We will not use ContextProviderCommandBuffer when using Vulkan. Maybe it should move to the if (!real_output_surface) block below? https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:635: cc::VulkanInProcessContextProvider::Create(); I think we would want to share this across multiple CompositorImpl (similar to GpuProcessTransportFactory::SharedVulkanContextProvider). Also, we added a command-line flag (kEnableVulkan), we should use it to decide whether or not to try to talk to Vulkan. https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:640: vulkan_surface.reset(new VulkanOutputSurface(vulkan_context_provider)); nit: std::move(vulkan_context_provider)
Please take a look, thanks. https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... File content/browser/renderer_host/compositor_impl_android.cc (right): https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:230: class VulkanOutputSurface : public cc::OutputSurface { On 2016/04/28 21:29:40, piman OOO back 2016-5-2 wrote: > I think this whole class needs to be inside #if defined(ENABLE_VULKAN), because > otherwise you won't have an implementation for VulkanSurface. Done. https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:233: const scoped_refptr<cc::VulkanContextProvider>& vulkan_context_provider) On 2016/04/28 21:29:40, piman OOO back 2016-5-2 wrote: > nit: scoped_refptr<cc::VulkanContextProvider> (not const scoped_refptr<>&), per > style guide. Done. https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:234: : OutputSurface(nullptr, nullptr, vulkan_context_provider, nullptr) {} On 2016/04/28 21:29:40, piman OOO back 2016-5-2 wrote: > nit: std::move(vulkan_context_provider) Done. https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:271: std::unique_ptr<gpu::VulkanSurface> surface_; On 2016/04/28 21:29:40, piman OOO back 2016-5-2 wrote: > nit: DISALLOW_COPY_AND_ASSIGN() Done. https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:632: DCHECK(context_provider.get()); On 2016/04/28 21:29:40, piman OOO back 2016-5-2 wrote: > All this code (from l.560 to here) should only execute when we don't use Vulkan. > We will not use ContextProviderCommandBuffer when using Vulkan. > Maybe it should move to the if (!real_output_surface) block below? Done. https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:635: cc::VulkanInProcessContextProvider::Create(); On 2016/04/28 21:29:40, piman OOO back 2016-5-2 wrote: > I think we would want to share this across multiple CompositorImpl (similar to > GpuProcessTransportFactory::SharedVulkanContextProvider). > > Also, we added a command-line flag (kEnableVulkan), we should use it to decide > whether or not to try to talk to Vulkan. Done. https://codereview.chromium.org/1921533008/diff/20001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:640: vulkan_surface.reset(new VulkanOutputSurface(vulkan_context_provider)); On 2016/04/28 21:29:40, piman OOO back 2016-5-2 wrote: > nit: std::move(vulkan_context_provider) Done.
Description was changed from ========== Add support for OutputSurface for Vulkan on Android. BUG=582558 ========== to ========== Add support for OutputSurface for Vulkan on Android. This adds an optional Vulkan Output Surface for Android's Browser Compositor,using Vulkan Context provider. BUG=582558 ==========
https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... File content/browser/renderer_host/compositor_impl_android.cc (right): https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:75: class VulkanSurface; nit: This is not necessary since you included the header.
https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... File content/browser/renderer_host/compositor_impl_android.cc (right): https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:782: CompositorImpl::SharedVulkanContextProviderAndroid() { I don't think this shares anything across CompositorImpls, unless it's static, and shared_vulkan_context_provider_android_ is static too. Keep in mind that shared_vulkan_context_provider_android_ can't be a global scoped_refptr (non-POD type), so you may want to use a LazyInstance or something.
https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... File content/browser/renderer_host/compositor_impl_android.cc (right): https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:782: CompositorImpl::SharedVulkanContextProviderAndroid() { On 2016/05/02 22:19:43, piman wrote: > I don't think this shares anything across CompositorImpls, unless it's static, > and shared_vulkan_context_provider_android_ is static too. > Keep in mind that > shared_vulkan_context_provider_android_ can't be a global scoped_refptr (non-POD > type), so you may want to use a LazyInstance or something. Hmm, if we are to make a lazyinstance (leaky or otherwise) of shared_vulkan_context_provider_android_, then we have to expose the ctor dtor publicly in VulkanInProcessContextProvider, and lose the ref-countedness? Is that ok ? Would we need to change GpuProcessTransportFactory's shared_vulkan_context_provider_ too ?
https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... File content/browser/renderer_host/compositor_impl_android.cc (right): https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:782: CompositorImpl::SharedVulkanContextProviderAndroid() { On 2016/05/03 12:38:53, sohanjg wrote: > On 2016/05/02 22:19:43, piman wrote: > > I don't think this shares anything across CompositorImpls, unless it's static, > > and shared_vulkan_context_provider_android_ is static too. > > Keep in mind that > > shared_vulkan_context_provider_android_ can't be a global scoped_refptr > (non-POD > > type), so you may want to use a LazyInstance or something. > > Hmm, if we are to make a lazyinstance (leaky or otherwise) of > shared_vulkan_context_provider_android_, then we have to expose the ctor dtor > publicly in VulkanInProcessContextProvider, and lose the ref-countedness? Is > that ok ? I was thinking LazyInstance<scoped_refptr<VulkanInProcessContextProvider>> > > Would we need to change GpuProcessTransportFactory's > shared_vulkan_context_provider_ too ? GpuProcessTransportFactory is already a singleton, which is shared among the compositors.
Please take a look, thanks. https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... File content/browser/renderer_host/compositor_impl_android.cc (right): https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:75: class VulkanSurface; On 2016/05/02 18:44:10, David Yen wrote: > nit: This is not necessary since you included the header. Done. https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... content/browser/renderer_host/compositor_impl_android.cc:782: CompositorImpl::SharedVulkanContextProviderAndroid() { On 2016/05/03 14:53:26, piman wrote: > On 2016/05/03 12:38:53, sohanjg wrote: > > On 2016/05/02 22:19:43, piman wrote: > > > I don't think this shares anything across CompositorImpls, unless it's > static, > > > and shared_vulkan_context_provider_android_ is static too. > > > Keep in mind that > > > shared_vulkan_context_provider_android_ can't be a global scoped_refptr > > (non-POD > > > type), so you may want to use a LazyInstance or something. > > > > Hmm, if we are to make a lazyinstance (leaky or otherwise) of > > shared_vulkan_context_provider_android_, then we have to expose the ctor dtor > > publicly in VulkanInProcessContextProvider, and lose the ref-countedness? Is > > that ok ? > > I was thinking LazyInstance<scoped_refptr<VulkanInProcessContextProvider>> > > > > > Would we need to change GpuProcessTransportFactory's > > shared_vulkan_context_provider_ too ? > > GpuProcessTransportFactory is already a singleton, which is shared among the > compositors. > Done. I was trying to use the cxt provider w/o scopedness.
Description was changed from ========== Add support for OutputSurface for Vulkan on Android. This adds an optional Vulkan Output Surface for Android's Browser Compositor,using Vulkan Context provider. BUG=582558 ========== to ========== Add support for OutputSurface for Vulkan on Android. This adds an optional Vulkan Output Surface for Android's Browser Compositor,using Vulkan Context provider. BUG=582558 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
On 2016/05/04 05:19:26, sohanjg wrote: > Please take a look, thanks. > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > File content/browser/renderer_host/compositor_impl_android.cc (right): > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > content/browser/renderer_host/compositor_impl_android.cc:75: class > VulkanSurface; > On 2016/05/02 18:44:10, David Yen wrote: > > nit: This is not necessary since you included the header. > > Done. > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > content/browser/renderer_host/compositor_impl_android.cc:782: > CompositorImpl::SharedVulkanContextProviderAndroid() { > On 2016/05/03 14:53:26, piman wrote: > > On 2016/05/03 12:38:53, sohanjg wrote: > > > On 2016/05/02 22:19:43, piman wrote: > > > > I don't think this shares anything across CompositorImpls, unless it's > > static, > > > > and shared_vulkan_context_provider_android_ is static too. > > > > Keep in mind that > > > > shared_vulkan_context_provider_android_ can't be a global scoped_refptr > > > (non-POD > > > > type), so you may want to use a LazyInstance or something. > > > > > > Hmm, if we are to make a lazyinstance (leaky or otherwise) of > > > shared_vulkan_context_provider_android_, then we have to expose the ctor > dtor > > > publicly in VulkanInProcessContextProvider, and lose the ref-countedness? Is > > > that ok ? > > > > I was thinking LazyInstance<scoped_refptr<VulkanInProcessContextProvider>> > > > > > > > > Would we need to change GpuProcessTransportFactory's > > > shared_vulkan_context_provider_ too ? > > > > GpuProcessTransportFactory is already a singleton, which is shared among the > > compositors. > > > > Done. > > I was trying to use the cxt provider w/o scopedness. It seems using lazyinstance of scopedrefptr in PS#4, was returning cxt provider as NULL. So, made it a leaky lazyinstance of VulkanInProcessContextProvider, and removed protectedness of VulkanInProcessContextProvider Ctor. Will this be allright ? Thanks.
On 2016/05/04 14:11:18, sohanjg wrote: > On 2016/05/04 05:19:26, sohanjg wrote: > > Please take a look, thanks. > > > > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > > File content/browser/renderer_host/compositor_impl_android.cc (right): > > > > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > > content/browser/renderer_host/compositor_impl_android.cc:75: class > > VulkanSurface; > > On 2016/05/02 18:44:10, David Yen wrote: > > > nit: This is not necessary since you included the header. > > > > Done. > > > > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > > content/browser/renderer_host/compositor_impl_android.cc:782: > > CompositorImpl::SharedVulkanContextProviderAndroid() { > > On 2016/05/03 14:53:26, piman wrote: > > > On 2016/05/03 12:38:53, sohanjg wrote: > > > > On 2016/05/02 22:19:43, piman wrote: > > > > > I don't think this shares anything across CompositorImpls, unless it's > > > static, > > > > > and shared_vulkan_context_provider_android_ is static too. > > > > > Keep in mind that > > > > > shared_vulkan_context_provider_android_ can't be a global scoped_refptr > > > > (non-POD > > > > > type), so you may want to use a LazyInstance or something. > > > > > > > > Hmm, if we are to make a lazyinstance (leaky or otherwise) of > > > > shared_vulkan_context_provider_android_, then we have to expose the ctor > > dtor > > > > publicly in VulkanInProcessContextProvider, and lose the ref-countedness? > Is > > > > that ok ? > > > > > > I was thinking LazyInstance<scoped_refptr<VulkanInProcessContextProvider>> > > > > > > > > > > > Would we need to change GpuProcessTransportFactory's > > > > shared_vulkan_context_provider_ too ? > > > > > > GpuProcessTransportFactory is already a singleton, which is shared among the > > > compositors. > > > > > > > Done. > > > > I was trying to use the cxt provider w/o scopedness. > > It seems using lazyinstance of scopedrefptr in PS#4, was returning cxt provider > as NULL. > So, made it a leaky lazyinstance of VulkanInProcessContextProvider, and removed > protectedness of VulkanInProcessContextProvider Ctor. > Will this be allright ? Thanks. I think g_shared_vulkan_context_provider_android_.Pointer(), will now invoke VulkanInProcessContextProvider Ctor, but our device queue is initialized thru VulkanInProcessContextProvider::Initialize, so need to invoke Initialize from Ctor too. Also, making the ctor public, may not be the best singleton design. Should we revert to PS#4 ? But there again we have cxt provider null issue. Pls let me know, whats the best option fwd. Thanks.
On Wed, May 4, 2016 at 7:11 AM, <sohan.jyoti@samsung.com> wrote: > On 2016/05/04 05:19:26, sohanjg wrote: > > Please take a look, thanks. > > > > > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > > File content/browser/renderer_host/compositor_impl_android.cc (right): > > > > > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > > content/browser/renderer_host/compositor_impl_android.cc:75: class > > VulkanSurface; > > On 2016/05/02 18:44:10, David Yen wrote: > > > nit: This is not necessary since you included the header. > > > > Done. > > > > > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > > content/browser/renderer_host/compositor_impl_android.cc:782: > > CompositorImpl::SharedVulkanContextProviderAndroid() { > > On 2016/05/03 14:53:26, piman wrote: > > > On 2016/05/03 12:38:53, sohanjg wrote: > > > > On 2016/05/02 22:19:43, piman wrote: > > > > > I don't think this shares anything across CompositorImpls, unless > it's > > > static, > > > > > and shared_vulkan_context_provider_android_ is static too. > > > > > Keep in mind that > > > > > shared_vulkan_context_provider_android_ can't be a global > scoped_refptr > > > > (non-POD > > > > > type), so you may want to use a LazyInstance or something. > > > > > > > > Hmm, if we are to make a lazyinstance (leaky or otherwise) of > > > > shared_vulkan_context_provider_android_, then we have to expose the > ctor > > dtor > > > > publicly in VulkanInProcessContextProvider, and lose the > ref-countedness? > Is > > > > that ok ? > > > > > > I was thinking > LazyInstance<scoped_refptr<VulkanInProcessContextProvider>> > > > > > > > > > > > Would we need to change GpuProcessTransportFactory's > > > > shared_vulkan_context_provider_ too ? > > > > > > GpuProcessTransportFactory is already a singleton, which is shared > among the > > > compositors. > > > > > > > Done. > > > > I was trying to use the cxt provider w/o scopedness. > > It seems using lazyinstance of scopedrefptr in PS#4, was returning cxt > provider > as NULL. > So, made it a leaky lazyinstance of VulkanInProcessContextProvider, and > removed > protectedness of VulkanInProcessContextProvider Ctor. > Will this be allright ? Thanks. > No no, not ok. PS#4 was going the right direction. It's just that you need to call VulkanInProcessContextProvider::Create in SharedVulkanContextProviderAndroid() when the LazyInstance is null (i.e. on the first call). > > https://codereview.chromium.org/1921533008/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/05/04 21:25:05, piman wrote: > On Wed, May 4, 2016 at 7:11 AM, <mailto:sohan.jyoti@samsung.com> wrote: > > > On 2016/05/04 05:19:26, sohanjg wrote: > > > Please take a look, thanks. > > > > > > > > > > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > > > File content/browser/renderer_host/compositor_impl_android.cc (right): > > > > > > > > > > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > > > content/browser/renderer_host/compositor_impl_android.cc:75: class > > > VulkanSurface; > > > On 2016/05/02 18:44:10, David Yen wrote: > > > > nit: This is not necessary since you included the header. > > > > > > Done. > > > > > > > > > > > https://codereview.chromium.org/1921533008/diff/40001/content/browser/rendere... > > > content/browser/renderer_host/compositor_impl_android.cc:782: > > > CompositorImpl::SharedVulkanContextProviderAndroid() { > > > On 2016/05/03 14:53:26, piman wrote: > > > > On 2016/05/03 12:38:53, sohanjg wrote: > > > > > On 2016/05/02 22:19:43, piman wrote: > > > > > > I don't think this shares anything across CompositorImpls, unless > > it's > > > > static, > > > > > > and shared_vulkan_context_provider_android_ is static too. > > > > > > Keep in mind that > > > > > > shared_vulkan_context_provider_android_ can't be a global > > scoped_refptr > > > > > (non-POD > > > > > > type), so you may want to use a LazyInstance or something. > > > > > > > > > > Hmm, if we are to make a lazyinstance (leaky or otherwise) of > > > > > shared_vulkan_context_provider_android_, then we have to expose the > > ctor > > > dtor > > > > > publicly in VulkanInProcessContextProvider, and lose the > > ref-countedness? > > Is > > > > > that ok ? > > > > > > > > I was thinking > > LazyInstance<scoped_refptr<VulkanInProcessContextProvider>> > > > > > > > > > > > > > > Would we need to change GpuProcessTransportFactory's > > > > > shared_vulkan_context_provider_ too ? > > > > > > > > GpuProcessTransportFactory is already a singleton, which is shared > > among the > > > > compositors. > > > > > > > > > > Done. > > > > > > I was trying to use the cxt provider w/o scopedness. > > > > It seems using lazyinstance of scopedrefptr in PS#4, was returning cxt > > provider > > as NULL. > > So, made it a leaky lazyinstance of VulkanInProcessContextProvider, and > > removed > > protectedness of VulkanInProcessContextProvider Ctor. > > Will this be allright ? Thanks. > > > > No no, not ok. PS#4 was going the right direction. It's just that you need > to call VulkanInProcessContextProvider::Create in > SharedVulkanContextProviderAndroid() when the LazyInstance is null (i.e. on > the first call). > Thanks. Reverted back to PS#4 with the ::Create change. It works fine now. Please take a look. > > > > https://codereview.chromium.org/1921533008/ > > > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org.
lgtm
Description was changed from ========== Add support for OutputSurface for Vulkan on Android. This adds an optional Vulkan Output Surface for Android's Browser Compositor,using Vulkan Context provider. BUG=582558 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== Add support for OutputSurface for Vulkan on Android. This adds an optional Vulkan Output Surface for Android's Browser Compositor,using a shared Vulkan Context provider. BUG=582558 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
The CQ bit was checked by sohan.jyoti@samsung.com
The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org Link to the patchset: https://codereview.chromium.org/1921533008/#ps120001 (title: "removed extra line breaks.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1921533008/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1921533008/120001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-device on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
On 2016/05/06 05:13:17, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > ios-device on tryserver.chromium.mac (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) > ios-device-gn on tryserver.chromium.mac (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) > ios-simulator on tryserver.chromium.mac (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) > mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) > mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...) > mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) rebased to ToT. Please take a look, thanks.
The CQ bit was checked by sohan.jyoti@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1921533008/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1921533008/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by sohan.jyoti@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1921533008/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1921533008/160001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1921533008/diff/160001/content/browser/render... File content/browser/renderer_host/compositor_impl_android.cc (left): https://codereview.chromium.org/1921533008/diff/160001/content/browser/render... content/browser/renderer_host/compositor_impl_android.cc:538: pending_swapbuffers_ = 0; I think this was lost in the rebase.
Please take a look, thanks. https://codereview.chromium.org/1921533008/diff/160001/content/browser/render... File content/browser/renderer_host/compositor_impl_android.cc (left): https://codereview.chromium.org/1921533008/diff/160001/content/browser/render... content/browser/renderer_host/compositor_impl_android.cc:538: pending_swapbuffers_ = 0; On 2016/05/06 17:17:29, piman wrote: > I think this was lost in the rebase. Done.
The CQ bit was checked by sohan.jyoti@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1921533008/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1921533008/180001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by sohan.jyoti@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1921533008/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1921533008/180001
Message was sent while issue was closed.
Description was changed from ========== Add support for OutputSurface for Vulkan on Android. This adds an optional Vulkan Output Surface for Android's Browser Compositor,using a shared Vulkan Context provider. BUG=582558 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== Add support for OutputSurface for Vulkan on Android. This adds an optional Vulkan Output Surface for Android's Browser Compositor,using a shared Vulkan Context provider. BUG=582558 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ==========
Message was sent while issue was closed.
Committed patchset #10 (id:180001)
Message was sent while issue was closed.
Description was changed from ========== Add support for OutputSurface for Vulkan on Android. This adds an optional Vulkan Output Surface for Android's Browser Compositor,using a shared Vulkan Context provider. BUG=582558 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel ========== to ========== Add support for OutputSurface for Vulkan on Android. This adds an optional Vulkan Output Surface for Android's Browser Compositor,using a shared Vulkan Context provider. BUG=582558 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/2986bd485401025eeef77025dd9697ec36f9d347 Cr-Commit-Position: refs/heads/master@{#392354} ==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/2986bd485401025eeef77025dd9697ec36f9d347 Cr-Commit-Position: refs/heads/master@{#392354} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
