Commit 4def961e authored by Juanmi Huertas's avatar Juanmi Huertas Committed by Commit Bot

Fixing TODOs of 1035589 in CanvasResourceProvider

There was two TODOs in CanvasResourceProvider of some checks that were
leftovers of the previous.

Both of them involved SharedBitmap resource provider. Both of them were
a check that the Create method was doing.

One of the tests is about checking we have a Dispatcher. That is needed
as inside the constructor of the CanvasResourceProvider of SharedBitmap
it has a DCHECK that it has a valid Dispatcher.

However the other check about the max texture size of the
WebGraphicsContext3dProviderWrapper is not needed, thus, I'm removing
that in this CL and simplifying the API.

Bug: 1035589
Change-Id: Id53f45c178b0ca5d8c143d411832731742e699e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225330Reviewed-by: default avatarYi Xu <yiyix@chromium.org>
Reviewed-by: default avatarFernando Serboncini <fserb@chromium.org>
Commit-Queue: Juanmi Huertas <juanmihd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775147}
parent 575238d6
......@@ -163,8 +163,7 @@ void CanvasRenderingContextHost::CreateCanvasResourceProvider3D(
// provider.
if (!provider) {
provider = CanvasResourceProvider::CreateSharedBitmapProvider(
Size(), SharedGpuContext::ContextProviderWrapper(), FilterQuality(),
ColorParams(), std::move(dispatcher));
Size(), FilterQuality(), ColorParams(), std::move(dispatcher));
}
if (!provider) {
provider = CanvasResourceProvider::CreateBitmapProvider(
......@@ -247,8 +246,7 @@ void CanvasRenderingContextHost::CreateCanvasResourceProvider2D(
// provider.
if (!provider) {
provider = CanvasResourceProvider::CreateSharedBitmapProvider(
Size(), SharedGpuContext::ContextProviderWrapper(), FilterQuality(),
ColorParams(), std::move(dispatcher));
Size(), FilterQuality(), ColorParams(), std::move(dispatcher));
}
if (!provider) {
provider = CanvasResourceProvider::CreateBitmapProvider(
......
......@@ -398,8 +398,7 @@ CanvasResourceProvider* OffscreenCanvas::GetOrCreateResourceProvider() {
base::WeakPtr<CanvasResourceDispatcher> dispatcher_weakptr =
GetOrCreateResourceDispatcher()->GetWeakPtr();
provider = CanvasResourceProvider::CreateSharedBitmapProvider(
surface_size, SharedGpuContext::ContextProviderWrapper(),
FilterQuality(), context_->ColorParams(),
surface_size, FilterQuality(), context_->ColorParams(),
std::move(dispatcher_weakptr));
}
......
......@@ -92,8 +92,8 @@ class CanvasResourceDispatcherTest
// kDefaultPresentationMode` created a sharedBitmap 100% of the time.
// Investigate study if the Bitmap fallback makes sense or not.
resource_provider_ = CanvasResourceProvider::CreateSharedBitmapProvider(
IntSize(kWidth, kHeight), nullptr /* context_provider_wrapper */,
kLow_SkFilterQuality, CanvasColorParams(), dispatcher_->GetWeakPtr());
IntSize(kWidth, kHeight), kLow_SkFilterQuality, CanvasColorParams(),
dispatcher_->GetWeakPtr());
if (!resource_provider_) {
resource_provider_ = CanvasResourceProvider::CreateBitmapProvider(
IntSize(kWidth, kHeight), kLow_SkFilterQuality, CanvasColorParams());
......
......@@ -741,26 +741,11 @@ CanvasResourceProvider::CreateBitmapProvider(
std::unique_ptr<CanvasResourceProvider>
CanvasResourceProvider::CreateSharedBitmapProvider(
const IntSize& size,
base::WeakPtr<WebGraphicsContext3DProviderWrapper> context_provider_wrapper,
SkFilterQuality filter_quality,
const CanvasColorParams& color_params,
base::WeakPtr<CanvasResourceDispatcher> resource_dispatcher) {
// TODO(crbug/1035589). The former CanvasResourceProvider::Create was doing
// this check as well for SharedBitmapProvider, while this should not make
// sense, will be left for a later CL to address this issue and the failing
// tests due to not having this check here.
if (SharedGpuContext::IsGpuCompositingEnabled() && context_provider_wrapper) {
const auto& max_texture_size = context_provider_wrapper->ContextProvider()
->GetCapabilities()
.max_texture_size;
if (size.Width() > max_texture_size || size.Height() > max_texture_size) {
return nullptr;
}
}
// TODO(crbug/1035589). The former CanvasResourceProvider::Create was doing
// this check as well for SharedBitmapProvider, as we are passing a weap_ptr
// maybe the caller could ensure an always valid weakptr.
// SharedBitmapProvider has to have a valid resource_dispatecher to be able to
// be created.
if (!resource_dispatcher)
return nullptr;
......
......@@ -89,7 +89,6 @@ class PLATFORM_EXPORT CanvasResourceProvider
static std::unique_ptr<CanvasResourceProvider> CreateSharedBitmapProvider(
const IntSize&,
base::WeakPtr<WebGraphicsContext3DProviderWrapper>,
SkFilterQuality,
const CanvasColorParams&,
base::WeakPtr<CanvasResourceDispatcher>);
......
......@@ -321,7 +321,7 @@ TEST_F(CanvasResourceProviderTest, CanvasResourceProviderSharedBitmap) {
1 /* placeholder_canvas_id */, kSize);
auto provider = CanvasResourceProvider::CreateSharedBitmapProvider(
kSize, context_provider_wrapper_, kLow_SkFilterQuality, kColorParams,
kSize, kLow_SkFilterQuality, kColorParams,
resource_dispatcher.GetWeakPtr());
EXPECT_EQ(provider->Size(), kSize);
......
......@@ -49,8 +49,7 @@ class FakeCanvasResourceHost : public CanvasResourceHost {
}
if (!provider) {
provider = CanvasResourceProvider::CreateSharedBitmapProvider(
size_, SharedGpuContext::ContextProviderWrapper(),
kMedium_SkFilterQuality, CanvasColorParams(),
size_, kMedium_SkFilterQuality, CanvasColorParams(),
nullptr /* dispatcher_weakptr */);
}
if (!provider) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment