Skip to content

Commit

Permalink
Store Option<ImageInfo> instead of making fields optional
Browse files Browse the repository at this point in the history
  • Loading branch information
@teapotd
teapotd committed Nov 1, 2019
1 parent 48d918d commit a4fa36f
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 69 deletions.
4 changes: 2 additions & 2 deletions components/script/dom/webgl_validations/tex_image_2d.rs
View file
Expand Up @@ -613,7 +613,7 @@ impl<'a> WebGLValidator for CompressedTexSubImage2DValidator<'a> {
compression,
} = self.compression_validator.validate()?;

let tex_info = texture.image_info_for_target(&target, level);
let tex_info = texture.image_info_for_target(&target, level).unwrap();

// GL_INVALID_VALUE is generated if:
// - xoffset or yoffset is less than 0
Expand All @@ -630,7 +630,7 @@ impl<'a> WebGLValidator for CompressedTexSubImage2DValidator<'a> {

// GL_INVALID_OPERATION is generated if format does not match
// internal_format.
if compression.format != tex_info.internal_format().unwrap() {
if compression.format != tex_info.internal_format() {
context.webgl_error(InvalidOperation);
return Err(TexImageValidationError::TextureFormatMismatch);
}
Expand Down
14 changes: 9 additions & 5 deletions components/script/dom/webglframebuffer.rs
View file
Expand Up @@ -221,12 +221,16 @@ impl WebGLFramebuffer {
Some(WebGLFramebufferAttachment::Texture {
texture: ref att_tex,
level,
}) => {
let info = att_tex.image_info_at_face(0, level as u32);
(
info.internal_format().map(|t| t.as_gl_constant()),
}) => match att_tex.image_info_at_face(0, level as u32) {
Some(info) => (
Some(info.internal_format().as_gl_constant()),
Some((info.width() as i32, info.height() as i32)),
)
),
None => {
self.status
.set(constants::FRAMEBUFFER_INCOMPLETE_ATTACHMENT);
return;
},
},
None => (None, None),
};
Expand Down
25 changes: 14 additions & 11 deletions components/script/dom/webglrenderingcontext.rs
View file
Expand Up @@ -438,13 +438,12 @@ impl WebGLRenderingContext {
}

let target = TexImageTarget::Texture2D;
let info = texture.image_info_for_target(&target, 0);
if info.is_initialized() {
if let Some(info) = texture.image_info_for_target(&target, 0) {
self.validate_filterable_texture(
&texture,
target,
0,
info.internal_format().unwrap_or(TexFormat::RGBA),
info.internal_format(),
Size2D::new(info.width(), info.height()),
info.data_type().unwrap_or(TexDataType::UnsignedByte),
);
Expand Down Expand Up @@ -746,7 +745,10 @@ impl WebGLRenderingContext {
pixels: TexPixels,
) {
// We have already validated level
let image_info = texture.image_info_for_target(&target, level);
let image_info = match texture.image_info_for_target(&target, level) {
Some(info) => info,
None => return self.webgl_error(InvalidOperation),
};

// GL_INVALID_VALUE is generated if:
// - xoffset or yoffset is less than 0
Expand All @@ -761,9 +763,7 @@ impl WebGLRenderingContext {
}

// NB: format and internal_format must match.
if format != image_info.internal_format().unwrap() ||
data_type != image_info.data_type().unwrap()
{
if format != image_info.internal_format() || data_type != image_info.data_type().unwrap() {
return self.webgl_error(InvalidOperation);
}

Expand Down Expand Up @@ -1921,9 +1921,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
Some(WebGLFramebufferAttachmentRoot::Renderbuffer(rb)) => {
TexFormat::from_gl_constant(rb.internal_format())
},
Some(WebGLFramebufferAttachmentRoot::Texture(texture)) => {
texture.image_info_for_target(&target, 0).internal_format()
},
Some(WebGLFramebufferAttachmentRoot::Texture(texture)) => texture
.image_info_for_target(&target, 0)
.map(|info| info.internal_format()),
None => None,
},
None => {
Expand Down Expand Up @@ -2022,7 +2022,10 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
Err(_) => return,
};

let image_info = texture.image_info_for_target(&target, level);
let image_info = match texture.image_info_for_target(&target, level) {
Some(info) => info,
None => return self.webgl_error(InvalidOperation),
};

// GL_INVALID_VALUE is generated if:
// - xoffset or yoffset is less than 0
Expand Down
73 changes: 22 additions & 51 deletions components/script/dom/webgltexture.rs
View file
Expand Up @@ -40,7 +40,7 @@ pub struct WebGLTexture {
is_deleted: Cell<bool>,
/// Stores information about mipmap levels and cubemap faces.
#[ignore_malloc_size_of = "Arrays are cumbersome"]
image_info_array: DomRefCell<[ImageInfo; MAX_LEVEL_COUNT * MAX_FACE_COUNT]>,
image_info_array: DomRefCell<[Option<ImageInfo>; MAX_LEVEL_COUNT * MAX_FACE_COUNT]>,
/// Face count can only be 1 or 6
face_count: Cell<u8>,
base_mipmap_level: u32,
Expand All @@ -64,7 +64,7 @@ impl WebGLTexture {
base_mipmap_level: 0,
min_filter: Cell::new(constants::NEAREST_MIPMAP_LINEAR),
mag_filter: Cell::new(constants::LINEAR),
image_info_array: DomRefCell::new([ImageInfo::new(); MAX_LEVEL_COUNT * MAX_FACE_COUNT]),
image_info_array: DomRefCell::new([None; MAX_LEVEL_COUNT * MAX_FACE_COUNT]),
attached_to_dom: Cell::new(false),
attached_framebuffer: Default::default(),
}
Expand Down Expand Up @@ -135,8 +135,7 @@ impl WebGLTexture {
width: width,
height: height,
depth: depth,
internal_format: Some(internal_format),
is_initialized: true,
internal_format: internal_format,
data_type: data_type,
};

Expand All @@ -159,10 +158,7 @@ impl WebGLTexture {
},
};

let base_image_info = self.base_image_info();
if !base_image_info.is_initialized() {
return Err(WebGLError::InvalidOperation);
}
let base_image_info = self.base_image_info().ok_or(WebGLError::InvalidOperation)?;

let is_cubic = target == constants::TEXTURE_CUBE_MAP;
if is_cubic && !self.is_cube_complete() {
Expand Down Expand Up @@ -308,10 +304,9 @@ impl WebGLTexture {
}

pub fn populate_mip_chain(&self, first_level: u32, last_level: u32) -> WebGLResult<()> {
let base_image_info = self.image_info_at_face(0, first_level);
if !base_image_info.is_initialized() {
return Err(WebGLError::InvalidOperation);
}
let base_image_info = self
.image_info_at_face(0, first_level)
.ok_or(WebGLError::InvalidOperation)?;

let mut ref_width = base_image_info.width;
let mut ref_height = base_image_info.height;
Expand All @@ -333,7 +328,6 @@ impl WebGLTexture {
height: ref_height,
depth: 0,
internal_format: base_image_info.internal_format,
is_initialized: base_image_info.is_initialized(),
data_type: base_image_info.data_type,
};

Expand All @@ -345,19 +339,19 @@ impl WebGLTexture {
fn is_cube_complete(&self) -> bool {
debug_assert_eq!(self.face_count.get(), 6);

let image_info = self.base_image_info();
if !image_info.is_defined() {
return false;
}
let image_info = match self.base_image_info() {
Some(info) => info,
None => return false,
};

let ref_width = image_info.width;
let ref_format = image_info.internal_format;

for face in 0..self.face_count.get() {
let current_image_info = self.image_info_at_face(face, self.base_mipmap_level);
if !current_image_info.is_defined() {
return false;
}
let current_image_info = match self.image_info_at_face(face, self.base_mipmap_level) {
Some(info) => info,
None => return false,
};

// Compares height with width to enforce square dimensions
if current_image_info.internal_format != ref_format ||
Expand All @@ -383,12 +377,12 @@ impl WebGLTexture {
}
}

pub fn image_info_for_target(&self, target: &TexImageTarget, level: u32) -> ImageInfo {
pub fn image_info_for_target(&self, target: &TexImageTarget, level: u32) -> Option<ImageInfo> {
let face_index = self.face_index_for_target(&target);
self.image_info_at_face(face_index, level)
}

pub fn image_info_at_face(&self, face: u8, level: u32) -> ImageInfo {
pub fn image_info_at_face(&self, face: u8, level: u32) -> Option<ImageInfo> {
let pos = (level * self.face_count.get() as u32) + face as u32;
self.image_info_array.borrow()[pos as usize]
}
Expand All @@ -402,10 +396,10 @@ impl WebGLTexture {
fn set_image_infos_at_level_and_face(&self, level: u32, face: u8, image_info: ImageInfo) {
debug_assert!(face < self.face_count.get());
let pos = (level * self.face_count.get() as u32) + face as u32;
self.image_info_array.borrow_mut()[pos as usize] = image_info;
self.image_info_array.borrow_mut()[pos as usize] = Some(image_info);
}

fn base_image_info(&self) -> ImageInfo {
fn base_image_info(&self) -> Option<ImageInfo> {
assert!((self.base_mipmap_level as usize) < MAX_LEVEL_COUNT);

self.image_info_at_face(0, self.base_mipmap_level)
Expand Down Expand Up @@ -435,23 +429,11 @@ pub struct ImageInfo {
width: u32,
height: u32,
depth: u32,
internal_format: Option<TexFormat>,
is_initialized: bool,
internal_format: TexFormat,
data_type: Option<TexDataType>,
}

impl ImageInfo {
fn new() -> ImageInfo {
ImageInfo {
width: 0,
height: 0,
depth: 0,
internal_format: None,
is_initialized: false,
data_type: None,
}
}

pub fn width(&self) -> u32 {
self.width
}
Expand All @@ -460,7 +442,7 @@ impl ImageInfo {
self.height
}

pub fn internal_format(&self) -> Option<TexFormat> {
pub fn internal_format(&self) -> TexFormat {
self.internal_format
}

Expand All @@ -474,14 +456,6 @@ impl ImageInfo {
self.depth.is_power_of_two()
}

pub fn is_initialized(&self) -> bool {
self.is_initialized
}

fn is_defined(&self) -> bool {
self.internal_format.is_some()
}

fn get_max_mimap_levels(&self) -> u32 {
let largest = cmp::max(cmp::max(self.width, self.height), self.depth);
if largest == 0 {
Expand All @@ -492,10 +466,7 @@ impl ImageInfo {
}

fn is_compressed_format(&self) -> bool {
match self.internal_format {
Some(format) => format.is_compressed(),
None => false,
}
self.internal_format.is_compressed()
}
}

Expand Down

0 comments on commit a4fa36f

Please sign in to comment.