Skip to content

Commit

Permalink
Revert 224117 "gpu: Upgrade DEPTH_COMPONENT16 to DEPTH_COMPONENT..."
Browse files Browse the repository at this point in the history 
Caused http://crbug.com/294937

> gpu: Upgrade DEPTH_COMPONENT16 to DEPTH_COMPONENT24 if possible
> 
> Upgrade the DEPTH_COMPONENT16 render buffer storage format to
> DEPTH_COMPONENT24 if the GLES implementation supports the OES_depth24
> extension. This is done to improve content portability between desktop
> and mobile, since we already do a similar mapping from DEPTH_COMPONENT16
> to the unsized DEPTH_COMPONENT on desktop GL. This means that for
> example WebGL content can end up relying on a higher precision depth
> buffer even though it only requested DEPTH_COMPONENT16.
> 
> BUG=285400
> TEST=http://www.khronos.org/registry/webgl/conformance-suites/1.0.1/webgl-conformance-tests.html on Nexus 4
> 
> Review URL: https://chromiumcodereview.appspot.com/24079010

TBR=skyostil@chromium.org

Review URL: https://codereview.chromium.org/23672048

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@224166 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
thakis@chromium.org committed Sep 19, 2013
1 parent 9e11ba5 commit 9493d9e
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 88 deletions.
5 changes: 1 addition & 4 deletions gpu/command_buffer/service/context_group.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,12 @@ bool ContextGroup::Initialize(
draw_buffer_ = GL_BACK;
}

const bool depth24_supported = feature_info_->feature_flags().oes_depth24;

buffer_manager_.reset(
new BufferManager(memory_tracker_.get(), feature_info_.get()));
framebuffer_manager_.reset(
new FramebufferManager(max_draw_buffers_, max_color_attachments_));
renderbuffer_manager_.reset(new RenderbufferManager(
memory_tracker_.get(), max_renderbuffer_size, max_samples,
depth24_supported));
memory_tracker_.get(), max_renderbuffer_size, max_samples));
shader_manager_.reset(new ShaderManager());

// Lookup GL things we need to know.
Expand Down
2 changes: 0 additions & 2 deletions gpu/command_buffer/service/feature_info.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ FeatureInfo::FeatureFlags::FeatureFlags()
use_img_for_multisampled_render_to_texture(false),
oes_standard_derivatives(false),
oes_egl_image_external(false),
oes_depth24(false),
npot_ok(false),
enable_texture_float_linear(false),
enable_texture_half_float_linear(false),
Expand Down Expand Up @@ -498,7 +497,6 @@ void FeatureInfo::InitializeFeatures() {

if (extensions.Contains("GL_OES_depth24") || gfx::HasDesktopGLFeatures()) {
AddExtensionString("GL_OES_depth24");
feature_flags_.oes_depth24 = true;
validators_.render_buffer_format.AddValue(GL_DEPTH_COMPONENT24);
}

Expand Down
1 change: 0 additions & 1 deletion gpu/command_buffer/service/feature_info.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class GPU_EXPORT FeatureInfo : public base::RefCounted<FeatureInfo> {
bool use_img_for_multisampled_render_to_texture;
bool oes_standard_derivatives;
bool oes_egl_image_external;
bool oes_depth24;
bool npot_ok;
bool enable_texture_float_linear;
bool enable_texture_half_float_linear;
Expand Down
35 changes: 22 additions & 13 deletions gpu/command_buffer/service/framebuffer_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,19 @@ using ::testing::Return;

namespace gpu {
namespace gles2 {
namespace {

const GLint kMaxTextureSize = 64;
const GLint kMaxCubemapSize = 64;
const GLint kMaxRenderbufferSize = 64;
const GLint kMaxSamples = 4;
const bool kDepth24Supported = false;

} // namespace

class FramebufferManagerTest : public testing::Test {
static const GLint kMaxTextureSize = 64;
static const GLint kMaxCubemapSize = 64;
static const GLint kMaxRenderbufferSize = 64;
static const GLint kMaxSamples = 4;

public:
FramebufferManagerTest()
: manager_(1, 1),
texture_manager_(
NULL, new FeatureInfo(), kMaxTextureSize, kMaxCubemapSize),
renderbuffer_manager_(NULL, kMaxRenderbufferSize, kMaxSamples,
kDepth24Supported) {
renderbuffer_manager_(NULL, kMaxRenderbufferSize, kMaxSamples) {

}
virtual ~FramebufferManagerTest() {
Expand All @@ -59,6 +54,13 @@ class FramebufferManagerTest : public testing::Test {
RenderbufferManager renderbuffer_manager_;
};

// GCC requires these declarations, but MSVC requires they not be present
#ifndef COMPILER_MSVC
const GLint FramebufferManagerTest::kMaxTextureSize;
const GLint FramebufferManagerTest::kMaxCubemapSize;
const GLint FramebufferManagerTest::kMaxRenderbufferSize;
#endif

TEST_F(FramebufferManagerTest, Basic) {
const GLuint kClient1Id = 1;
const GLuint kService1Id = 11;
Expand Down Expand Up @@ -108,12 +110,16 @@ class FramebufferInfoTest : public testing::Test {
static const GLuint kClient1Id = 1;
static const GLuint kService1Id = 11;

static const GLint kMaxTextureSize = 64;
static const GLint kMaxCubemapSize = 64;
static const GLint kMaxRenderbufferSize = 64;
static const GLint kMaxSamples = 4;

FramebufferInfoTest()
: manager_(1, 1),
texture_manager_(
NULL, new FeatureInfo(), kMaxTextureSize, kMaxCubemapSize),
renderbuffer_manager_(NULL, kMaxRenderbufferSize, kMaxSamples,
kDepth24Supported) {
renderbuffer_manager_(NULL, kMaxRenderbufferSize, kMaxSamples) {
}
virtual ~FramebufferInfoTest() {
manager_.Destroy(false);
Expand Down Expand Up @@ -149,6 +155,9 @@ class FramebufferInfoTest : public testing::Test {
#ifndef COMPILER_MSVC
const GLuint FramebufferInfoTest::kClient1Id;
const GLuint FramebufferInfoTest::kService1Id;
const GLint FramebufferInfoTest::kMaxTextureSize;
const GLint FramebufferInfoTest::kMaxCubemapSize;
const GLint FramebufferInfoTest::kMaxRenderbufferSize;
#endif

TEST_F(FramebufferInfoTest, Basic) {
Expand Down
25 changes: 11 additions & 14 deletions gpu/command_buffer/service/gles2_cmd_decoder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1941,8 +1941,8 @@ bool BackRenderbuffer::AllocateStorage(const gfx::Size& size, GLenum format,
ScopedRenderBufferBinder binder(decoder_, id_);

uint32 estimated_size = 0;
if (!decoder_->renderbuffer_manager()->ComputeEstimatedRenderbufferSize(
size.width(), size.height(), samples, format, &estimated_size)) {
if (!RenderbufferManager::ComputeEstimatedRenderbufferSize(
size.width(), size.height(), samples, format, &estimated_size)) {
return false;
}

Expand Down Expand Up @@ -5022,8 +5022,8 @@ void GLES2DecoderImpl::DoRenderbufferStorageMultisample(
}

uint32 estimated_size = 0;
if (!renderbuffer_manager()->ComputeEstimatedRenderbufferSize(
width, height, samples, internalformat, &estimated_size)) {
if (!RenderbufferManager::ComputeEstimatedRenderbufferSize(
width, height, samples, internalformat, &estimated_size)) {
LOCAL_SET_GL_ERROR(
GL_OUT_OF_MEMORY,
"glRenderbufferStorageMultsample", "dimensions too large");
Expand All @@ -5037,9 +5037,8 @@ void GLES2DecoderImpl::DoRenderbufferStorageMultisample(
return;
}

GLenum impl_format =
renderbuffer_manager()->InternalRenderbufferFormatToImplFormat(
internalformat);
GLenum impl_format = RenderbufferManager::
InternalRenderbufferFormatToImplFormat(internalformat);
LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("glRenderbufferStorageMultisample");
if (IsAngle()) {
glRenderbufferStorageMultisampleANGLE(
Expand Down Expand Up @@ -5191,8 +5190,8 @@ void GLES2DecoderImpl::DoRenderbufferStorage(
}

uint32 estimated_size = 0;
if (!renderbuffer_manager()->ComputeEstimatedRenderbufferSize(
width, height, 1, internalformat, &estimated_size)) {
if (!RenderbufferManager::ComputeEstimatedRenderbufferSize(
width, height, 1, internalformat, &estimated_size)) {
LOCAL_SET_GL_ERROR(
GL_OUT_OF_MEMORY, "glRenderbufferStorage", "dimensions too large");
return;
Expand All @@ -5206,11 +5205,9 @@ void GLES2DecoderImpl::DoRenderbufferStorage(

LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER("glRenderbufferStorage");
glRenderbufferStorageEXT(
target,
renderbuffer_manager()->InternalRenderbufferFormatToImplFormat(
internalformat),
width,
height);
target, RenderbufferManager::
InternalRenderbufferFormatToImplFormat(internalformat),
width, height);
GLenum error = LOCAL_PEEK_GL_ERROR("glRenderbufferStorage");
if (error == GL_NO_ERROR) {
// TODO(gman): If tetxures tracked which framebuffers they were attached to
Expand Down
19 changes: 5 additions & 14 deletions gpu/command_buffer/service/renderbuffer_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ namespace gles2 {
RenderbufferManager::RenderbufferManager(
MemoryTracker* memory_tracker,
GLint max_renderbuffer_size,
GLint max_samples,
bool depth24_supported)
GLint max_samples)
: memory_tracker_(
new MemoryTypeTracker(memory_tracker, MemoryTracker::kUnmanaged)),
max_renderbuffer_size_(max_renderbuffer_size),
max_samples_(max_samples),
depth24_supported_(depth24_supported),
num_uncleared_renderbuffers_(0),
renderbuffer_count_(0),
have_context_(true) {
Expand All @@ -40,7 +38,7 @@ RenderbufferManager::~RenderbufferManager() {

size_t Renderbuffer::EstimatedSize() {
uint32 size = 0;
manager_->ComputeEstimatedRenderbufferSize(
RenderbufferManager::ComputeEstimatedRenderbufferSize(
width_, height_, samples_, internal_format_, &size);
return size;
}
Expand Down Expand Up @@ -151,11 +149,8 @@ void RenderbufferManager::RemoveRenderbuffer(GLuint client_id) {
}
}

bool RenderbufferManager::ComputeEstimatedRenderbufferSize(int width,
int height,
int samples,
int internal_format,
uint32* size) const {
bool RenderbufferManager::ComputeEstimatedRenderbufferSize(
int width, int height, int samples, int internal_format, uint32* size) {
DCHECK(size);

uint32 temp = 0;
Expand All @@ -175,7 +170,7 @@ bool RenderbufferManager::ComputeEstimatedRenderbufferSize(int width,
}

GLenum RenderbufferManager::InternalRenderbufferFormatToImplFormat(
GLenum impl_format) const {
GLenum impl_format) {
if (gfx::GetGLImplementation() != gfx::kGLImplementationEGLGLES2) {
switch (impl_format) {
case GL_DEPTH_COMPONENT16:
Expand All @@ -186,10 +181,6 @@ GLenum RenderbufferManager::InternalRenderbufferFormatToImplFormat(
case GL_RGB565:
return GL_RGB;
}
} else {
// Upgrade 16-bit depth to 24-bit if possible.
if (impl_format == GL_DEPTH_COMPONENT16 && depth24_supported_)
return GL_DEPTH_COMPONENT24;
}
return impl_format;
}
Expand Down
13 changes: 4 additions & 9 deletions gpu/command_buffer/service/renderbuffer_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ class GPU_EXPORT RenderbufferManager {
public:
RenderbufferManager(MemoryTracker* memory_tracker,
GLint max_renderbuffer_size,
GLint max_samples,
bool depth24_supported);
GLint max_samples);
~RenderbufferManager();

GLint max_renderbuffer_size() const {
Expand Down Expand Up @@ -164,12 +163,9 @@ class GPU_EXPORT RenderbufferManager {
return memory_tracker_->GetMemRepresented();
}

bool ComputeEstimatedRenderbufferSize(int width,
int height,
int samples,
int internal_format,
uint32* size) const;
GLenum InternalRenderbufferFormatToImplFormat(GLenum impl_format) const;
static bool ComputeEstimatedRenderbufferSize(
int width, int height, int samples, int internal_format, uint32* size);
static GLenum InternalRenderbufferFormatToImplFormat(GLenum impl_format);

private:
friend class Renderbuffer;
Expand All @@ -181,7 +177,6 @@ class GPU_EXPORT RenderbufferManager {

GLint max_renderbuffer_size_;
GLint max_samples_;
bool depth24_supported_;

int num_uncleared_renderbuffers_;

Expand Down
37 changes: 6 additions & 31 deletions gpu/command_buffer/service/renderbuffer_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "gpu/command_buffer/common/gles2_cmd_utils.h"
#include "gpu/command_buffer/service/mocks.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gl/gl_implementation.h"
#include "ui/gl/gl_mock.h"

using ::testing::StrictMock;
Expand All @@ -22,11 +21,11 @@ class RenderbufferManagerTestBase : public testing::Test {
static const GLint kMaxSamples = 4;

protected:
void SetUpBase(MemoryTracker* memory_tracker, bool depth24_supported) {
void SetUpBase(MemoryTracker* memory_tracker) {
gl_.reset(new ::testing::StrictMock<gfx::MockGLInterface>());
::gfx::GLInterface::SetGLInterface(gl_.get());
manager_.reset(new RenderbufferManager(
memory_tracker, kMaxSize, kMaxSamples, depth24_supported));
memory_tracker, kMaxSize, kMaxSamples));
}

virtual void TearDown() {
Expand All @@ -44,8 +43,7 @@ class RenderbufferManagerTestBase : public testing::Test {
class RenderbufferManagerTest : public RenderbufferManagerTestBase {
protected:
virtual void SetUp() {
bool depth24_supported = false;
SetUpBase(NULL, depth24_supported);
SetUpBase(NULL);
}
};

Expand All @@ -54,8 +52,7 @@ class RenderbufferManagerMemoryTrackerTest
protected:
virtual void SetUp() {
mock_memory_tracker_ = new StrictMock<MockMemoryTracker>();
bool depth24_supported = false;
SetUpBase(mock_memory_tracker_.get(), depth24_supported);
SetUpBase(mock_memory_tracker_.get());
}

scoped_refptr<MockMemoryTracker> mock_memory_tracker_;
Expand Down Expand Up @@ -182,9 +179,9 @@ TEST_F(RenderbufferManagerMemoryTrackerTest, Basic) {
const GLsizei kHeight2 = 32;
uint32 expected_size_1 = 0;
uint32 expected_size_2 = 0;
manager_->ComputeEstimatedRenderbufferSize(
RenderbufferManager::ComputeEstimatedRenderbufferSize(
kWidth, kHeight1, kSamples, kFormat, &expected_size_1);
manager_->ComputeEstimatedRenderbufferSize(
RenderbufferManager::ComputeEstimatedRenderbufferSize(
kWidth, kHeight2, kSamples, kFormat, &expected_size_2);
EXPECT_MEMORY_ALLOCATION_CHANGE(
0, expected_size_1, MemoryTracker::kUnmanaged);
Expand Down Expand Up @@ -294,28 +291,6 @@ TEST_F(RenderbufferManagerTest, AddToSignature) {
.RetiresOnSaturation();
}

class RenderbufferManagerFormatTest : public RenderbufferManagerTestBase {
protected:
virtual void SetUp() {
bool depth24_supported = true;
SetUpBase(NULL, depth24_supported);
}
};

TEST_F(RenderbufferManagerFormatTest, UpgradeDepthFormatOnGLES) {
gfx::SetGLImplementation(gfx::kGLImplementationEGLGLES2);
GLenum impl_format =
manager_->InternalRenderbufferFormatToImplFormat(GL_DEPTH_COMPONENT16);
EXPECT_EQ(static_cast<GLenum>(GL_DEPTH_COMPONENT24), impl_format);
}

TEST_F(RenderbufferManagerFormatTest, UseUnsizedDepthFormatOnNonGLES) {
gfx::SetGLImplementation(gfx::kGLImplementationDesktopGL);
GLenum impl_format =
manager_->InternalRenderbufferFormatToImplFormat(GL_DEPTH_COMPONENT16);
EXPECT_EQ(static_cast<GLenum>(GL_DEPTH_COMPONENT), impl_format);
}

} // namespace gles2
} // namespace gpu

Expand Down

0 comments on commit 9493d9e

Please sign in to comment.