Skip to content

Commit

Permalink
Don't allow rendering with uninitialized GL_TEXTURE_EXTERNAL_OES text…
Browse files Browse the repository at this point in the history
…ures

Texture::CompatibleWithSamplerUniformType only expects to run with
"renderable" textures, where the base level is initialized - in
particular it expects a valid internal_format/format/type.
Some old logic was skipping a check for GL_TEXTURE_EXTERNAL_OES,
allowing textures for which we didn't know the size, but that ends up
allowing uninitialized textures. Nowadays we should always know the size
even for those, so remove the logic skipping the initialized level check.

Bug: 966936
Change-Id: I61f64fe4fa7551ed7640f41fc0ea1111edb641bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1629792
Auto-Submit: Antoine Labour <piman@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663896}
  • Loading branch information
pimanttr authored and Commit Bot committed May 28, 2019
1 parent c2c83d6 commit c47ea48
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 16 deletions.
1 change: 1 addition & 0 deletions gpu/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ test("gl_tests") {
"command_buffer/tests/gl_tests_main.cc",
"command_buffer/tests/gl_texture_mailbox_unittest.cc",
"command_buffer/tests/gl_texture_storage_unittest.cc",
"command_buffer/tests/gl_unallocated_texture_unittest.cc",
"command_buffer/tests/gl_unittest.cc",
"command_buffer/tests/gl_unittests_android.cc",
"command_buffer/tests/gl_virtual_contexts_ext_window_rectangles_unittest.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3870,8 +3870,8 @@ TEST_P(GLES2DecoderManualInitTest, DrawWithGLImageExternal) {
scoped_refptr<MockGLImage> image(new MockGLImage);
group().texture_manager()->SetTarget(texture_ref, GL_TEXTURE_EXTERNAL_OES);
group().texture_manager()->SetLevelInfo(texture_ref, GL_TEXTURE_EXTERNAL_OES,
0, GL_RGBA, 0, 0, 1, 0, GL_RGBA,
GL_UNSIGNED_BYTE, gfx::Rect());
0, GL_RGBA, 1, 1, 1, 0, GL_RGBA,
GL_UNSIGNED_BYTE, gfx::Rect(1, 1));
group().texture_manager()->SetLevelImage(texture_ref, GL_TEXTURE_EXTERNAL_OES,
0, image.get(), Texture::BOUND);

Expand Down
21 changes: 9 additions & 12 deletions gpu/command_buffer/service/texture_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -690,18 +690,15 @@ Texture::CanRenderCondition Texture::GetCanRenderCondition() const {
if (target_ == 0)
return CAN_RENDER_ALWAYS;

if (target_ != GL_TEXTURE_EXTERNAL_OES) {
if (face_infos_.empty() ||
static_cast<size_t>(base_level_) >= face_infos_[0].level_infos.size()) {
return CAN_RENDER_NEVER;
}
const Texture::LevelInfo& first_face =
face_infos_[0].level_infos[base_level_];
if (first_face.width == 0 ||
first_face.height == 0 ||
first_face.depth == 0) {
return CAN_RENDER_NEVER;
}
if (face_infos_.empty() ||
static_cast<size_t>(base_level_) >= face_infos_[0].level_infos.size()) {
return CAN_RENDER_NEVER;
}
const Texture::LevelInfo& first_face =
face_infos_[0].level_infos[base_level_];
if (first_face.width == 0 || first_face.height == 0 ||
first_face.depth == 0) {
return CAN_RENDER_NEVER;
}

if (target_ == GL_TEXTURE_CUBE_MAP && !cube_complete())
Expand Down
17 changes: 15 additions & 2 deletions gpu/command_buffer/service/texture_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -756,12 +756,12 @@ TEST_F(TextureTest, SetTargetTextureExternalOES) {
EXPECT_FALSE(TextureTestHelper::IsCubeComplete(texture));
EXPECT_FALSE(manager_->CanGenerateMipmaps(texture_ref_.get()));
EXPECT_TRUE(TextureTestHelper::IsNPOT(texture));
EXPECT_TRUE(manager_->CanRender(texture_ref_.get()));
EXPECT_FALSE(manager_->CanRender(texture_ref_.get()));
EXPECT_TRUE(texture->SafeToRenderFrom());
EXPECT_TRUE(texture->IsImmutable());
}

TEST_F(TextureTest, ZeroSizeCanNotRender) {
TEST_F(TextureTest, ZeroSizeCanNotRender2D) {
manager_->SetTarget(texture_ref_.get(), GL_TEXTURE_2D);
EXPECT_FALSE(manager_->CanRender(texture_ref_.get()));
manager_->SetLevelInfo(texture_ref_.get(), GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 1,
Expand All @@ -772,6 +772,19 @@ TEST_F(TextureTest, ZeroSizeCanNotRender) {
EXPECT_FALSE(manager_->CanRender(texture_ref_.get()));
}

TEST_F(TextureTest, ZeroSizeCanNotRenderExternalOES) {
manager_->SetTarget(texture_ref_.get(), GL_TEXTURE_EXTERNAL_OES);
EXPECT_FALSE(manager_->CanRender(texture_ref_.get()));
manager_->SetLevelInfo(texture_ref_.get(), GL_TEXTURE_EXTERNAL_OES, 0,
GL_RGBA, 1, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE,
gfx::Rect(1, 1));
EXPECT_TRUE(manager_->CanRender(texture_ref_.get()));
manager_->SetLevelInfo(texture_ref_.get(), GL_TEXTURE_EXTERNAL_OES, 0,
GL_RGBA, 0, 0, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE,
gfx::Rect());
EXPECT_FALSE(manager_->CanRender(texture_ref_.get()));
}

TEST_F(TextureTest, CanRenderTo) {
TestHelper::SetupFeatureInfoInitExpectations(gl_.get(), "");
scoped_refptr<FeatureInfo> feature_info(new FeatureInfo());
Expand Down
120 changes: 120 additions & 0 deletions gpu/command_buffer/tests/gl_unallocated_texture_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <GLES2/gl2.h>
#include <GLES2/gl2ext.h>
#include <stddef.h>
#include <stdint.h>

#include "gpu/GLES2/gl2extchromium.h"
#include "gpu/command_buffer/client/gles2_implementation.h"
#include "gpu/command_buffer/client/gles2_lib.h"
#include "gpu/command_buffer/service/gles2_cmd_decoder.h"
#include "gpu/command_buffer/tests/gl_manager.h"
#include "gpu/command_buffer/tests/gl_test_utils.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gl/gl_share_group.h"

namespace gpu {

class GLUnallocatedTextureTest : public testing::Test {
protected:
void SetUp() override { gl_.Initialize(GLManager::Options()); }

void TearDown() override { gl_.Destroy(); }

GLuint MakeProgram(const char* fragment_shader) {
constexpr const char kVertexShader[] =
"void main() { gl_Position = vec4(0.0, 0.0, 0.0, 1.0); }";
GLuint program = GLTestHelper::LoadProgram(kVertexShader, fragment_shader);
if (!program)
return 0;
glUseProgram(program);

GLint location_sampler = glGetUniformLocation(program, "sampler");
glUniform1i(location_sampler, 0);
return program;
}

// Creates a texture on target, setting up filters but without setting any
// level image.
GLuint MakeUninitializedTexture(GLenum target) {
GLuint texture = 0;
glGenTextures(1, &texture);
glBindTexture(target, texture);
glTexParameteri(target, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
glTexParameteri(target, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
glTexParameteri(target, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
glTexParameteri(target, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
return texture;
}

GLManager gl_;
};

// Test that we can render with GL_TEXTURE_2D textures that are unallocated.
// This should not generate errors or assert.
TEST_F(GLUnallocatedTextureTest, RenderUnallocatedTexture2D) {
constexpr const char kFragmentShader[] =
"uniform sampler2D sampler;\n"
"void main() { gl_FragColor = texture2D(sampler, vec2(0.0, 0.0)); }\n";
GLuint program = MakeProgram(kFragmentShader);
ASSERT_TRUE(program);
GLuint texture = MakeUninitializedTexture(GL_TEXTURE_2D);

glDrawArrays(GL_TRIANGLES, 0, 3);
EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());

glDeleteTextures(1, &texture);
glDeleteProgram(program);
}

// Test that we can render with GL_TEXTURE_EXTERNAL_OES textures that are
// unallocated. This should not generate errors or assert.
TEST_F(GLUnallocatedTextureTest, RenderUnallocatedTextureExternal) {
if (!gl_.GetCapabilities().egl_image_external) {
LOG(INFO) << "GL_OES_EGL_image_external not supported, skipping test";
return;
}
constexpr const char kFragmentShader[] =
"#extension GL_OES_EGL_image_external : enable\n"
"uniform samplerExternalOES sampler;\n"
"void main() { gl_FragColor = texture2D(sampler, vec2(0.0, 0.0)); }\n";
GLuint program = MakeProgram(kFragmentShader);
ASSERT_TRUE(program);
GLuint texture = MakeUninitializedTexture(GL_TEXTURE_EXTERNAL_OES);

glDrawArrays(GL_TRIANGLES, 0, 3);
EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());

glDeleteTextures(1, &texture);
glDeleteProgram(program);
}

// Test that we can render with GL_TEXTURE_RECTANGLE_ARB textures that are
// unallocated. This should not generate errors or assert.
TEST_F(GLUnallocatedTextureTest, RenderUnallocatedTextureRectange) {
if (!gl_.GetCapabilities().texture_rectangle) {
LOG(INFO) << "GL_ARB_texture_rectangle not supported, skipping test";
return;
}
constexpr const char kFragmentShader[] =
"#extension GL_ARB_texture_rectangle : enable\n"
"uniform sampler2DRect sampler;\n"
"void main() {\n"
" gl_FragColor = texture2DRect(sampler, vec2(0.0, 0.0));\n"
"}\n";
GLuint program = MakeProgram(kFragmentShader);
ASSERT_TRUE(program);
GLuint texture = MakeUninitializedTexture(GL_TEXTURE_RECTANGLE_ARB);

glDrawArrays(GL_TRIANGLES, 0, 3);
EXPECT_EQ(static_cast<GLenum>(GL_NO_ERROR), glGetError());

glDeleteTextures(1, &texture);
glDeleteProgram(program);
}

} // namespace gpu

0 comments on commit c47ea48

Please sign in to comment.