Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use an intermediate renderbuffer when blitting between texture arrays… #3327

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -857,6 +857,11 @@ pub struct Device {
/// format, we fall back to glTexImage*.
texture_storage_usage: TexStorageUsage,

// Whether we are able to use glBlitFramebuffers with the draw fbo
// bound to a non-0th layer of a texture array. This is buggy on
// adreno devices.
can_blit_to_texture_array: bool,

// GL extensions
extensions: Vec<String>,
}
@@ -1024,6 +1029,10 @@ impl Device {
)
};

// Due to a bug on adreno devices, blitting to an fbo bound to
// a non-0th layer of a texture array is not supported.
let can_blit_to_texture_array = !renderer_name.starts_with("Adreno");

// Explicitly set some global states to the values we expect.
gl.disable(gl::FRAMEBUFFER_SRGB);
gl.disable(gl::MULTISAMPLE);
@@ -1066,6 +1075,7 @@ impl Device {
frame_id: GpuFrameId(0),
extensions,
texture_storage_usage,
can_blit_to_texture_array,
}
}

@@ -1621,12 +1631,47 @@ impl Device {
debug_assert!(dst.size.height >= src.size.height);
debug_assert!(dst.layer_count >= src.layer_count);

// Note that zip() truncates to the shorter of the two iterators.
// If we are unable to blit directly between the texture arrays
// then we must create an intermediate renderbuffer.
let temp_buffer = if src.layer_count > 1 && !self.can_blit_to_texture_array {
let rbo = RBOId(self.gl.gen_renderbuffers(1)[0]);
let fbo = FBOId(self.gl.gen_framebuffers(1)[0]);
self.gl.bind_renderbuffer(gl::RENDERBUFFER, rbo.0);
self.gl.renderbuffer_storage(gl::RENDERBUFFER, gl::RGBA8,

This comment has been minimized.

Copy link
@kvark

kvark Nov 20, 2018

Member

how are we sure gl::RGBA8 is compatible with whatever texture we are blitting?

src.size.width as _, src.size.height as _);
self.bind_draw_target_impl(fbo);
self.gl.framebuffer_renderbuffer(gl::DRAW_FRAMEBUFFER,
gl::COLOR_ATTACHMENT0,
gl::RENDERBUFFER,
rbo.0);

Some((rbo, fbo))
} else {
None
};

let rect = DeviceIntRect::new(DeviceIntPoint::zero(), src.get_dimensions().to_i32());
for (read_fbo, draw_fbo) in src.fbos.iter().zip(&dst.fbos) {
for (i, read_fbo) in src.fbos.iter().enumerate() {
// Blit from the src to the dst (or the intermediate buffer if necessary)
self.bind_read_target_impl(*read_fbo);
self.bind_draw_target_impl(*draw_fbo);
self.bind_draw_target_impl(match &temp_buffer {

This comment has been minimized.

Copy link
@kvark

kvark Nov 20, 2018

Member

nit: remove &

Some((_, fbo)) => *fbo,
None => dst.fbos[i],
});
self.blit_render_target(rect, rect);

// Copy from the intermediate buffer to the dst texture if necessary
if let Some((_, fbo)) = &temp_buffer {

This comment has been minimized.

Copy link
@kvark

kvark Nov 20, 2018

Member

here and below: when matching we don't need the reference, instead we can avoid the fields moved out by marking them as Some((_, ref fbo)). IIRC, recent Rust inserts some of those ref automatically, but I'm not a big fan of that

self.bind_read_target_impl(*fbo);
self.bind_texture(DEFAULT_TEXTURE, dst);
self.gl.copy_tex_sub_image_3d(dst.target, 0, 0, 0, i as _,

This comment has been minimized.

Copy link
@kvark

kvark Nov 20, 2018

Member

note that we have 2 bugs here covered by the same flag: inability to blit to layer != 0 and inability to copy from layer != 0. The flag name itself only reflects the first bug, which is a bit unclear.

0, 0, src.size.width as _, src.size.height as _);
self.gl.invalidate_framebuffer(gl::READ_FRAMEBUFFER, &[gl::COLOR_ATTACHMENT0]);
}
}
if let Some((rbo, fbo)) = &temp_buffer {
self.gl.delete_renderbuffers(&[rbo.0]);
self.gl.delete_framebuffers(&[fbo.0]);
}
self.reset_draw_target();
self.reset_read_target();
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.