Skip to content

Commit

Permalink
Cleanup ContextCreationAttribs
Browse files Browse the repository at this point in the history
Bunch of attributes are not used anymore:
* buffer_preserved is not read, nothing sets it to not default value
* single_buffer nothing sets it to not default value
* samples/sample_buffers/stencil_size/depth_size are default inited to
  -1 and only overridden to 0. Command decoders check if they are > 0,
  so that code is never reached.
* gpu::Capabilities::num_stencil_bits is not used.
* red/green/blue_bits are used only on android [1] and alpha_size is
  set to only on android.

This CL removes unused attributes and code that sets it in the client.
Immediate affected code from GLES2 decoders is also removed, to make
this compile, but extensive cleanup will be done in follow-up.

Color attributes are not really needed on Android either, but it's not
trivial clean up, so for now put them under BUILDFLAG(IS_ANDROID).

[1] https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:gpu/ipc/service/gles2_command_buffer_stub.cc;l=140-144;drc=b8a0323a84f483b25e94b3a24d80fda16c5dd1ae;bpv=0;bpt=1

Bug: 1445523
Change-Id: Icc4d63922eff5b2b4702692316ac3ab3c21bd4aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4582013
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1154072}
  • Loading branch information
vasilyt authored and Chromium LUCI CQ committed Jun 6, 2023
1 parent c4f7638 commit a82fbde
Show file tree
Hide file tree
Showing 21 changed files with 56 additions and 229 deletions.
8 changes: 0 additions & 8 deletions chrome/browser/vr/win/graphics_delegate_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@ bool GraphicsDelegateWin::InitializeOnMainThread() {
scoped_refptr<gpu::GpuChannelHost> host = factory->EstablishGpuChannelSync();

gpu::ContextCreationAttribs attributes;
attributes.alpha_size = -1;
attributes.red_size = 8;
attributes.green_size = 8;
attributes.blue_size = 8;
attributes.stencil_size = 0;
attributes.depth_size = 0;
attributes.samples = 0;
attributes.sample_buffers = 0;
attributes.bind_generates_resource = false;

context_provider_ = base::MakeRefCounted<viz::ContextProviderCommandBuffer>(
Expand Down
4 changes: 0 additions & 4 deletions components/webxr/android/openxr_device_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,6 @@ void OpenXrDeviceProvider::CreateContextProviderAsync(
attributes.red_size = 8;
attributes.green_size = 8;
attributes.blue_size = 8;
attributes.stencil_size = 0;
attributes.depth_size = 0;
attributes.samples = 0;
attributes.sample_buffers = 0;
attributes.bind_generates_resource = false;
if (base::SysInfo::IsLowEndDevice()) {
attributes.alpha_size = 0;
Expand Down
4 changes: 0 additions & 4 deletions components/webxr/mailbox_to_surface_bridge_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,6 @@ void MailboxToSurfaceBridgeImpl::CreateAndBindContextProvider(
attributes.red_size = 8;
attributes.green_size = 8;
attributes.blue_size = 8;
attributes.stencil_size = 0;
attributes.depth_size = 0;
attributes.samples = 0;
attributes.sample_buffers = 0;
attributes.bind_generates_resource = false;
if (base::SysInfo::IsLowEndDevice()) {
attributes.alpha_size = 0;
Expand Down
6 changes: 0 additions & 6 deletions content/browser/compositor/viz_process_transport_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,8 @@ scoped_refptr<viz::ContextProviderCommandBuffer> CreateContextProvider(
constexpr bool kAutomaticFlushes = false;

gpu::ContextCreationAttribs attributes;
attributes.alpha_size = -1;
attributes.depth_size = 0;
attributes.stencil_size = 0;
attributes.samples = 0;
attributes.sample_buffers = 0;
attributes.bind_generates_resource = false;
attributes.lose_context_when_out_of_memory = true;
attributes.buffer_preserved = false;
attributes.enable_gles2_interface = supports_gles2_interface;
attributes.enable_raster_interface = supports_raster_interface;
attributes.enable_oop_rasterization = supports_gpu_rasterization;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ void OnGpuChannelEstablished(
attributes.red_size = 8;
attributes.green_size = 8;
attributes.blue_size = 8;
attributes.stencil_size = 0;
attributes.depth_size = 0;
attributes.samples = 0;
attributes.sample_buffers = 0;
attributes.bind_generates_resource = false;
attributes.enable_raster_interface = true;

Expand Down
4 changes: 0 additions & 4 deletions content/browser/renderer_host/compositor_impl_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,6 @@ gpu::ContextCreationAttribs GetCompositorContextAttributes(
// background.
gpu::ContextCreationAttribs attributes;
attributes.alpha_size = -1;
attributes.stencil_size = 0;
attributes.depth_size = 0;
attributes.samples = 0;
attributes.sample_buffers = 0;
attributes.bind_generates_resource = false;
attributes.color_space = gpu::COLOR_SPACE_SRGB;

Expand Down
5 changes: 0 additions & 5 deletions content/renderer/render_thread_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -338,11 +338,6 @@ scoped_refptr<viz::ContextProviderCommandBuffer> CreateOffscreenContext(
// This is for an offscreen context, so the default framebuffer doesn't need
// alpha, depth, stencil, antialiasing.
gpu::ContextCreationAttribs attributes;
attributes.alpha_size = -1;
attributes.depth_size = 0;
attributes.stencil_size = 0;
attributes.samples = 0;
attributes.sample_buffers = 0;
attributes.bind_generates_resource = false;
attributes.lose_context_when_out_of_memory = true;
attributes.enable_gles2_interface = support_gles2_interface;
Expand Down
5 changes: 0 additions & 5 deletions content/renderer/renderer_blink_platform_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -665,11 +665,6 @@ RendererBlinkPlatformImpl::CreateOffscreenGraphicsContext3DProvider(
Collect3DContextInformation(gl_info, gpu_channel_host->gpu_info());

gpu::ContextCreationAttribs attributes;
attributes.alpha_size = -1;
attributes.depth_size = 0;
attributes.stencil_size = 0;
attributes.samples = 0;
attributes.sample_buffers = 0;
attributes.bind_generates_resource = false;
attributes.enable_raster_interface = web_attributes.enable_raster_interface;
attributes.enable_oop_rasterization =
Expand Down
1 change: 0 additions & 1 deletion gpu/command_buffer/common/capabilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ struct GPU_EXPORT Capabilities {
int max_viewport_height = 0;
int num_compressed_texture_formats = 0;
int num_shader_binary_formats = 0;
int num_stencil_bits = 0; // For the default framebuffer.
int bind_generates_resource_chromium = 0;

int max_3d_texture_size = 0;
Expand Down
10 changes: 3 additions & 7 deletions gpu/command_buffer/common/context_creation_attribs.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <stdint.h>

#include "build/build_config.h"
#include "gpu/gpu_export.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gl/gpu_preference.h"
Expand Down Expand Up @@ -48,19 +49,14 @@ struct GPU_EXPORT ContextCreationAttribs {
gl::GpuPreference gpu_preference = gl::GpuPreference::kLowPower;
// -1 if invalid or unspecified.
int32_t alpha_size = -1;
#if BUILDFLAG(IS_ANDROID)
int32_t blue_size = -1;
int32_t green_size = -1;
int32_t red_size = -1;
int32_t depth_size = -1;
int32_t stencil_size = -1;
int32_t samples = -1;
int32_t sample_buffers = -1;
bool buffer_preserved = true;
#endif
bool bind_generates_resource = true;
bool fail_if_major_perf_caveat = false;
bool lose_context_when_out_of_memory = false;
bool should_use_native_gmb_for_backbuffer = false;
bool single_buffer = false;
bool enable_gles2_interface = true;
bool enable_grcontext = false;
bool enable_raster_interface = false;
Expand Down
109 changes: 35 additions & 74 deletions gpu/command_buffer/service/gles2_cmd_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3606,16 +3606,7 @@ gpu::ContextResult GLES2DecoderImpl::Initialize(
}
CHECK_GL_ERROR();

should_use_native_gmb_for_backbuffer_ =
attrib_helper.should_use_native_gmb_for_backbuffer;

#if !BUILDFLAG(IS_OZONE)
if (should_use_native_gmb_for_backbuffer_) {
LOG(ERROR) << "ContextResult::kFatalFailure: "
"native gmb format not supported";
return gpu::ContextResult::kFatalFailure;
}
#endif
should_use_native_gmb_for_backbuffer_ = false;

// In theory |needs_emulation| needs to be true on Desktop GL 4.1 or lower.
// However, we set it to true everywhere, not to trust drivers to handle
Expand Down Expand Up @@ -3738,20 +3729,12 @@ gpu::ContextResult GLES2DecoderImpl::Initialize(
GLint alpha_bits = 0;

if (offscreen) {
#if BUILDFLAG(IS_ANDROID)
offscreen_buffer_should_have_alpha_ = attrib_helper.alpha_size > 0;

if (attrib_helper.samples > 0 && attrib_helper.sample_buffers > 0 &&
features().chromium_framebuffer_multisample) {
// Per ext_framebuffer_multisample spec, need max bound on sample count.
// max_sample_count must be initialized to a sane value. If
// glGetIntegerv() throws a GL error, it leaves its argument unchanged.
GLint max_sample_count = 0;
api()->glGetIntegervFn(GL_MAX_SAMPLES_EXT, &max_sample_count);
offscreen_target_samples_ = std::min(attrib_helper.samples,
max_sample_count);
} else {
offscreen_target_samples_ = 0;
}
#else
offscreen_buffer_should_have_alpha_ = false;
#endif
offscreen_target_samples_ = 0;

if (gl_version_info().is_es) {
const bool rgb8_supported = features().oes_rgb8_rgba8;
Expand All @@ -3776,18 +3759,9 @@ gpu::ContextResult GLES2DecoderImpl::Initialize(
feature_info_->feature_flags().packed_depth24_stencil8;
VLOG(1) << "GL_OES_packed_depth_stencil "
<< (depth24_stencil8_supported ? "" : "not ") << "supported.";
if ((attrib_helper.depth_size > 0 || attrib_helper.stencil_size > 0) &&
depth24_stencil8_supported) {
offscreen_target_depth_format_ = GL_DEPTH24_STENCIL8;
offscreen_target_stencil_format_ = 0;
} else {
// It may be the case that this depth/stencil combination is not
// supported, but this will be checked later by CheckFramebufferStatus.
offscreen_target_depth_format_ = attrib_helper.depth_size > 0 ?
GL_DEPTH_COMPONENT16 : 0;
offscreen_target_stencil_format_ = attrib_helper.stencil_size > 0 ?
GL_STENCIL_INDEX8 : 0;
}

offscreen_target_depth_format_ = 0;
offscreen_target_stencil_format_ = 0;
} else {
offscreen_target_color_format_ =
offscreen_buffer_should_have_alpha_ ||
Expand All @@ -3803,16 +3777,8 @@ gpu::ContextResult GLES2DecoderImpl::Initialize(
VLOG(1) << "GL_EXT_packed_depth_stencil "
<< (depth24_stencil8_supported ? "" : "not ") << "supported.";

if ((attrib_helper.depth_size > 0 || attrib_helper.stencil_size > 0) &&
depth24_stencil8_supported) {
offscreen_target_depth_format_ = GL_DEPTH24_STENCIL8;
offscreen_target_stencil_format_ = 0;
} else {
offscreen_target_depth_format_ = attrib_helper.depth_size > 0 ?
GL_DEPTH_COMPONENT : 0;
offscreen_target_stencil_format_ = attrib_helper.stencil_size > 0 ?
GL_STENCIL_INDEX : 0;
}
offscreen_target_depth_format_ = 0;
offscreen_target_stencil_format_ = 0;
}

offscreen_saved_color_format_ = offscreen_buffer_should_have_alpha_ ||
Expand Down Expand Up @@ -3874,7 +3840,7 @@ gpu::ContextResult GLES2DecoderImpl::Initialize(
back_buffer_has_stencil_ = stencil_bits > 0;
num_stencil_bits_ = stencil_bits;
} else {
num_stencil_bits_ = attrib_helper.stencil_size;
num_stencil_bits_ = 0;
}

state_.viewport_width = surface->GetSize().width();
Expand Down Expand Up @@ -3959,14 +3925,12 @@ gpu::ContextResult GLES2DecoderImpl::Initialize(
std::make_unique<BackRenderbuffer>(this);
offscreen_target_stencil_render_buffer_->Create();

if (!attrib_helper.single_buffer) {
// Create the saved offscreen texture. The target frame buffer is copied
// here when SwapBuffers is called.
offscreen_saved_frame_buffer_ = std::make_unique<BackFramebuffer>(this);
offscreen_saved_frame_buffer_->Create();
offscreen_saved_color_texture_ = std::make_unique<BackTexture>(this);
offscreen_saved_color_texture_->Create();
}
// Create the saved offscreen texture. The target frame buffer is copied
// here when SwapBuffers is called.
offscreen_saved_frame_buffer_ = std::make_unique<BackFramebuffer>(this);
offscreen_saved_frame_buffer_->Create();
offscreen_saved_color_texture_ = std::make_unique<BackTexture>(this);
offscreen_saved_color_texture_->Create();

// Allocate the render buffers at their initial size and check the status
// of the frame buffers is okay.
Expand All @@ -3976,25 +3940,24 @@ gpu::ContextResult GLES2DecoderImpl::Initialize(
"Could not allocate offscreen buffer storage.";
return gpu::ContextResult::kFatalFailure;
}
if (!attrib_helper.single_buffer) {
// Allocate the offscreen saved color texture.
DCHECK(offscreen_saved_color_format_);
// Use 64x64 instead of 1x1 to handle minimum framebuffer size
// requirement on some platforms: b/151774454
offscreen_saved_color_texture_->AllocateStorage(
gfx::Size(64, 64), offscreen_saved_color_format_, true);

offscreen_saved_frame_buffer_->AttachRenderTexture(
offscreen_saved_color_texture_.get());
if (offscreen_saved_frame_buffer_->CheckStatus() !=
GL_FRAMEBUFFER_COMPLETE) {
bool was_lost = CheckResetStatus();
LOG(ERROR) << (was_lost ? "ContextResult::kTransientFailure: "
: "ContextResult::kFatalFailure: ")
<< "Offscreen saved FBO was incomplete.";
return was_lost ? gpu::ContextResult::kTransientFailure
: gpu::ContextResult::kFatalFailure;
}
// Allocate the offscreen saved color texture.
DCHECK(offscreen_saved_color_format_);
// Use 64x64 instead of 1x1 to handle minimum framebuffer size
// requirement on some platforms: b/151774454
offscreen_saved_color_texture_->AllocateStorage(
gfx::Size(64, 64), offscreen_saved_color_format_, true);

offscreen_saved_frame_buffer_->AttachRenderTexture(
offscreen_saved_color_texture_.get());
if (offscreen_saved_frame_buffer_->CheckStatus() !=
GL_FRAMEBUFFER_COMPLETE) {
bool was_lost = CheckResetStatus();
LOG(ERROR) << (was_lost ? "ContextResult::kTransientFailure: "
: "ContextResult::kFatalFailure: ")
<< "Offscreen saved FBO was incomplete.";
return was_lost ? gpu::ContextResult::kTransientFailure
: gpu::ContextResult::kFatalFailure;
}
}

Expand Down Expand Up @@ -4189,8 +4152,6 @@ Capabilities GLES2DecoderImpl::GetCapabilities() {
caps.max_samples = ComputeMaxSamples();
}

caps.num_stencil_bits = num_stencil_bits_;

caps.egl_image_external =
feature_info_->feature_flags().oes_egl_image_external;
caps.egl_image_external_essl3 =
Expand Down
33 changes: 4 additions & 29 deletions gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1245,18 +1245,12 @@ gpu::ContextResult GLES2DecoderPassthroughImpl::Initialize(
std::min(max_2d_texture_size, max_renderbuffer_size_);

if (offscreen_) {
const bool multisampled_framebuffers_supported =
feature_info_->feature_flags().chromium_framebuffer_multisample;
if (attrib_helper.samples > 0 && attrib_helper.sample_buffers > 0 &&
multisampled_framebuffers_supported && !attrib_helper.single_buffer) {
GLint max_sample_count = 0;
api()->glGetIntegervFn(GL_MAX_SAMPLES_EXT, &max_sample_count);
emulated_default_framebuffer_format_.samples =
std::min(attrib_helper.samples, max_sample_count);
}

const bool rgb8_supported = feature_info_->feature_flags().oes_rgb8_rgba8;
#if BUILDFLAG(IS_ANDROID)
const bool alpha_channel_requested = attrib_helper.alpha_size > 0;
#else
const bool alpha_channel_requested = false;
#endif
// The only available default render buffer formats in GLES2 have very
// little precision. Don't enable multisampling unless 8-bit render
// buffer formats are available--instead fall back to 8-bit textures.
Expand All @@ -1273,25 +1267,6 @@ gpu::ContextResult GLES2DecoderPassthroughImpl::Initialize(
emulated_default_framebuffer_format_.color_texture_internal_format;
emulated_default_framebuffer_format_.color_texture_type = GL_UNSIGNED_BYTE;

const bool depth24_stencil8_supported =
feature_info_->feature_flags().packed_depth24_stencil8;
if ((attrib_helper.depth_size > 0 || attrib_helper.stencil_size > 0) &&
depth24_stencil8_supported) {
emulated_default_framebuffer_format_.depth_stencil_internal_format =
GL_DEPTH24_STENCIL8;
} else {
// It may be the case that this depth/stencil combination is not
// supported, but this will be checked later by CheckFramebufferStatus.
if (attrib_helper.depth_size > 0) {
emulated_default_framebuffer_format_.depth_internal_format =
GL_DEPTH_COMPONENT16;
}
if (attrib_helper.stencil_size > 0) {
emulated_default_framebuffer_format_.stencil_internal_format =
GL_STENCIL_INDEX8;
}
}

CheckErrorCallbackState();
emulated_back_buffer_ = std::make_unique<EmulatedDefaultFramebuffer>(this);
// If we're an offscreen surface with zero width and/or height, set to a
Expand Down
6 changes: 2 additions & 4 deletions gpu/command_buffer/tests/fuzzer_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -231,15 +231,13 @@ class BitIterator {
struct Config {
size_t MakeFromBits(const uint8_t* bits, size_t size) {
BitIterator it(bits, size);
#if BUILDFLAG(IS_ANDROID)
attrib_helper.red_size = 8;
attrib_helper.green_size = 8;
attrib_helper.blue_size = 8;
attrib_helper.alpha_size = it.GetBit() ? 8 : 0;
attrib_helper.depth_size = it.GetBit() ? 24 : 0;
attrib_helper.stencil_size = it.GetBit() ? 8 : 0;
attrib_helper.buffer_preserved = it.GetBit();
#endif
attrib_helper.bind_generates_resource = it.GetBit();
attrib_helper.single_buffer = it.GetBit();
[[maybe_unused]] bool es3 = it.GetBit();
#if defined(GPU_FUZZER_USE_RASTER_DECODER)
attrib_helper.context_type = CONTEXT_TYPE_OPENGLES2;
Expand Down
1 change: 0 additions & 1 deletion gpu/gles2_conform_support/egl/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,6 @@ bool Context::CreateService(gl::GLSurface* gl_surface) {
CHECK_EQ(stencil_size, 0);

gpu::ContextCreationAttribs helper;
helper.buffer_preserved = false;
helper.bind_generates_resource = kBindGeneratesResources;
helper.fail_if_major_perf_caveat = false;
helper.lose_context_when_out_of_memory = kLoseContextWhenOutOfMemory;
Expand Down

0 comments on commit a82fbde

Please sign in to comment.