Skip to content

Commit 003dbeb

Browse files
gw3583gw3583
committed
Address some review comments.
1 parent 5f295b6 commit 003dbeb

File tree

6 files changed

+59
-180
lines changed

6 files changed

+59
-180
lines changed

‎sample/src/main.rs

Lines changed: 2 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,8 @@ use euclid::{Size2D, Point2D, Rect, Matrix4D};
88
use gleam::gl;
99
use std::path::PathBuf;
1010
use std::ffi::CStr;
11-
use webrender::{ExternalImage, ExternalImageData, ExternalImageHandler};
12-
use webrender_traits::{AuxiliaryListsBuilder, ColorF, Epoch, GlyphInstance, ExternalImageId};
13-
use webrender_traits::{ImageData, ImageFormat, PipelineId, RendererKind, ImageRendering};
11+
use webrender_traits::{AuxiliaryListsBuilder, ColorF, Epoch, GlyphInstance};
12+
use webrender_traits::{ImageData, ImageFormat, PipelineId, RendererKind};
1413
use std::fs::File;
1514
use std::io::Read;
1615
use std::env;
@@ -54,50 +53,6 @@ impl webrender_traits::RenderNotifier for Notifier {
5453
}
5554
}
5655

57-
struct ImageHandler {
58-
data: Vec<u8>,
59-
frame: u64,
60-
}
61-
62-
impl ImageHandler {
63-
fn new() -> ImageHandler {
64-
ImageHandler {
65-
frame: 0,
66-
data: vec![ 0xff, 0x00, 0x00, 0xff,
67-
0x00, 0xff, 0x00, 0xff,
68-
0x00, 0x00, 0xff, 0xff,
69-
0x00, 0xff, 0xff, 0xff ],
70-
}
71-
}
72-
}
73-
74-
impl ExternalImageHandler for ImageHandler {
75-
fn get(&mut self, _: ExternalImageId) -> ExternalImage {
76-
self.frame += 1;
77-
78-
if self.frame == 10 {
79-
self.data = vec![ 0x00, 0x00, 0x00, 0xff,
80-
0xff, 0xff, 0xff, 0xff,
81-
0xff, 0xff, 0xff, 0xff,
82-
0x00, 0x00, 0x00, 0xff ];
83-
}
84-
85-
ExternalImage {
86-
timestamp: self.frame,
87-
u0: 0.0,
88-
v0: 0.0,
89-
u1: 2.0,
90-
v1: 2.0,
91-
data: ExternalImageData::Buffer(self.data.as_ptr(),
92-
self.data.len())
93-
}
94-
}
95-
96-
fn release(&mut self, _: ExternalImageId) {
97-
98-
}
99-
}
100-
10156
fn main() {
10257
let args: Vec<String> = env::args().collect();
10358
if args.len() != 2 {
@@ -152,7 +107,6 @@ fn main() {
152107

153108
let notifier = Box::new(Notifier::new(window.create_window_proxy()));
154109
renderer.set_render_notifier(notifier);
155-
renderer.set_external_image_handler(Box::new(ImageHandler::new()));
156110

157111
let pipeline_id = PipelineId(0, 0);
158112
let epoch = Epoch(0);
@@ -202,22 +156,6 @@ fn main() {
202156

203157
let _text_bounds = Rect::new(Point2D::new(100.0, 200.0), Size2D::new(700.0, 300.0));
204158

205-
let ext_id = ExternalImageId(0);
206-
let image_key = api.add_image(2, 2, None, ImageFormat::RGBA8, ImageData::External(ext_id));
207-
208-
let ext_image_rect = Rect::new(Point2D::new(300.0, 100.0), Size2D::new(100.0, 100.0));
209-
let ext_image_clip_region = webrender_traits::ClipRegion::new(&ext_image_rect,
210-
vec![],
211-
None,
212-
&mut auxiliary_lists_builder);
213-
214-
builder.push_image(ext_image_rect,
215-
ext_image_clip_region,
216-
Size2D::new(100.0, 100.0),
217-
Size2D::zero(),
218-
ImageRendering::Auto,
219-
image_key);
220-
221159
let _glyphs = vec![
222160
GlyphInstance {
223161
index: 48,

‎webrender/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,5 +121,5 @@ extern crate offscreen_gl_context;
121121
extern crate byteorder;
122122
extern crate rayon;
123123

124-
pub use renderer::{ExternalImage, ExternalImageData, ExternalImageHandler};
124+
pub use renderer::{ExternalImage, ExternalImageSource, ExternalImageHandler};
125125
pub use renderer::{Renderer, RendererOptions};

‎webrender/src/prim_store.rs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,7 @@ impl PrimitiveStore {
620620
PrimitiveKind::Image => {
621621
let image_cpu = &mut self.cpu_images[metadata.cpu_prim_index.0];
622622

623-
match image_cpu.kind {
623+
let (texture_id, cache_item) = match image_cpu.kind {
624624
ImagePrimitiveKind::Image(image_key, image_rendering, _) => {
625625
// Check if an external image that needs to be resolved
626626
// by the render thread.
@@ -635,25 +635,27 @@ impl PrimitiveStore {
635635
resource_address: image_cpu.resource_address,
636636
image_properties: image_properties,
637637
});
638-
image_cpu.color_texture_id = SourceTexture::External(external_id);
638+
639+
(SourceTexture::External(external_id), None)
639640
}
640641
None => {
641642
let cache_item = resource_cache.get_cached_image(image_key, image_rendering);
642-
let resource_rect = self.gpu_resource_rects.get_mut(image_cpu.resource_address);
643-
resource_rect.uv0 = cache_item.uv0;
644-
resource_rect.uv1 = cache_item.uv1;
645-
image_cpu.color_texture_id = cache_item.texture_id;
643+
(cache_item.texture_id, Some(cache_item))
646644
}
647645
}
648646
}
649647
ImagePrimitiveKind::WebGL(context_id) => {
650648
let cache_item = resource_cache.get_webgl_texture(&context_id);
651-
let resource_rect = self.gpu_resource_rects.get_mut(image_cpu.resource_address);
652-
resource_rect.uv0 = cache_item.uv0;
653-
resource_rect.uv1 = cache_item.uv1;
654-
image_cpu.color_texture_id = cache_item.texture_id;
649+
(cache_item.texture_id, Some(cache_item))
655650
}
651+
};
652+
653+
if let Some(cache_item) = cache_item {
654+
let resource_rect = self.gpu_resource_rects.get_mut(image_cpu.resource_address);
655+
resource_rect.uv0 = cache_item.uv0;
656+
resource_rect.uv1 = cache_item.uv1;
656657
}
658+
image_cpu.color_texture_id = texture_id;
657659
}
658660
}
659661
}

‎webrender/src/render_backend.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::io::{Cursor, Read};
1616
use std::sync::{Arc, Mutex};
1717
use std::sync::mpsc::Sender;
1818
use texture_cache::TextureCache;
19-
use webrender_traits::{ApiMsg, AuxiliaryLists, BuiltDisplayList, IdNamespace};
19+
use webrender_traits::{ApiMsg, AuxiliaryLists, BuiltDisplayList, IdNamespace, ImageData};
2020
use webrender_traits::{FlushNotifier, RenderNotifier, RenderDispatcher, WebGLCommand, WebGLContextId};
2121
use record;
2222
use tiling::FrameBuilderConfig;
@@ -125,11 +125,8 @@ impl RenderBackend {
125125
tx.send(glyph_dimensions).unwrap();
126126
}
127127
ApiMsg::AddImage(id, width, height, stride, format, data) => {
128-
match data {
129-
ImageData::Raw(ref bytes) => {
130-
profile_counters.image_templates.inc(bytes.len());
131-
}
132-
ImageData::External(..) => {}
128+
if let ImageData::Raw(ref bytes) = data {
129+
profile_counters.image_templates.inc(bytes.len());
133130
}
134131
self.resource_cache.add_image_template(id,
135132
width,

‎webrender/src/renderer.rs

Lines changed: 39 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,10 @@ use profiler::{Profiler, BackendProfileCounters};
2424
use profiler::{GpuProfileTag, RendererProfileTimers, RendererProfileCounters};
2525
use render_backend::RenderBackend;
2626
use std::cmp;
27-
use std::collections::hash_map::Entry;
2827
use std::collections::HashMap;
2928
use std::f32;
3029
use std::hash::BuildHasherDefault;
31-
use std::{mem, slice};
30+
use std::mem;
3231
use std::path::PathBuf;
3332
use std::sync::{Arc, Mutex};
3433
use std::sync::mpsc::{channel, Receiver, Sender};
@@ -39,7 +38,7 @@ use tiling::{RenderTarget, ClearTile};
3938
use time::precise_time_ns;
4039
use util::TransformedRectKind;
4140
use webrender_traits::{ColorF, Epoch, FlushNotifier, PipelineId, RenderNotifier, RenderDispatcher};
42-
use webrender_traits::{ImageFormat, RenderApiSender, RendererKind};
41+
use webrender_traits::{ExternalImageId, ImageFormat, RenderApiSender, RendererKind};
4342

4443
pub const MAX_VERTEX_TEXTURE_WIDTH: usize = 1024;
4544

@@ -63,13 +62,6 @@ const GPU_TAG_PRIM_BORDER: GpuProfileTag = GpuProfileTag { label: "Border", colo
6362
const GPU_TAG_PRIM_CACHE_IMAGE: GpuProfileTag = GpuProfileTag { label: "CacheImage", color: debug_colors::SILVER };
6463
const GPU_TAG_BLUR: GpuProfileTag = GpuProfileTag { label: "Blur", color: debug_colors::VIOLET };
6564

66-
/// Internal details regarding external textures that the
67-
/// renderer knows about.
68-
struct ExternalTexture {
69-
timestamp: u64,
70-
texture_id: TextureId,
71-
}
72-
7365
#[derive(Debug, Copy, Clone, PartialEq)]
7466
pub enum BlendMode {
7567
None,
@@ -416,7 +408,7 @@ pub struct Renderer {
416408
external_image_handler: Option<Box<ExternalImageHandler>>,
417409

418410
/// Map of external image IDs to native textures.
419-
external_images: HashMap<ExternalImageId, ExternalTexture, BuildHasherDefault<FnvHasher>>,
411+
external_images: HashMap<ExternalImageId, TextureId, BuildHasherDefault<FnvHasher>>,
420412
}
421413

422414
impl Renderer {
@@ -822,10 +814,9 @@ impl Renderer {
822814
&SourceTexture::Invalid => TextureId::invalid(),
823815
&SourceTexture::WebGL(id) => TextureId::new(id),
824816
&SourceTexture::External(ref key) => {
825-
self.external_images
826-
.get(key)
827-
.expect("BUG: External image should be resolved by now!")
828-
.texture_id
817+
*self.external_images
818+
.get(key)
819+
.expect("BUG: External image should be resolved by now!")
829820
}
830821
&SourceTexture::TextureCache(index) => {
831822
self.cache_texture_id_map[index.0]
@@ -1353,9 +1344,7 @@ impl Renderer {
13531344
self.device.set_blend(false);
13541345
}
13551346

1356-
fn draw_tile_frame(&mut self,
1357-
frame: &mut Frame,
1358-
framebuffer_size: &Size2D<u32>) {
1347+
fn update_deferred_resolves(&mut self, frame: &mut Frame) {
13591348
// The first thing we do is run through any pending deferred
13601349
// resolves, and use a callback to get the UV rect for this
13611350
// custom item. Then we patch the resource_rects structure
@@ -1366,89 +1355,40 @@ impl Renderer {
13661355
.expect("Found external image, but no handler set!");
13671356

13681357
for deferred_resolve in &frame.deferred_resolves {
1369-
// First, check if we need to update this external image.
1370-
// If we haven't got this image previously, or if the
1371-
// version of the image has changed, then we will ask
1372-
// for the image and upload it to the GPU.
13731358
let props = &deferred_resolve.image_properties;
13741359
let external_id = props.external_id
13751360
.expect("BUG: Deferred resolves must be external images!");
13761361
let image = handler.get(external_id);
1377-
let texture_id = match self.external_images.entry(external_id) {
1378-
Entry::Occupied(mut entry) => {
1379-
let current_info = entry.get_mut();
1380-
if image.timestamp != current_info.timestamp {
1381-
current_info.timestamp = image.timestamp;
1382-
Some(current_info.texture_id)
1383-
} else {
1384-
None
1385-
}
1386-
}
1387-
Entry::Vacant(entry) => {
1388-
let texture_id = match image.data {
1389-
ExternalImageData::Buffer(..) => {
1390-
// For a custom user buffer, allocate a native texture.
1391-
let texture_id = self.device
1392-
.create_texture_ids(1, TextureTarget::Default)[0];
1393-
self.device.init_texture(texture_id,
1394-
props.width,
1395-
props.height,
1396-
props.format,
1397-
TextureFilter::Linear,
1398-
RenderTargetMode::None,
1399-
None);
1400-
texture_id
1401-
}
1402-
ExternalImageData::Native(texture_id) => {
1403-
// User has supplied a native texture, so
1404-
// just use it!
1405-
TextureId::new(texture_id)
1406-
}
1407-
};
14081362

1409-
entry.insert(ExternalTexture {
1410-
texture_id: texture_id,
1411-
timestamp: image.timestamp,
1412-
});
1413-
1414-
Some(texture_id)
1415-
}
1363+
let texture_id = match image.source {
1364+
ExternalImageSource::NativeTexture(texture_id) => TextureId::new(texture_id),
14161365
};
14171366

1418-
if let Some(texture_id) = texture_id {
1419-
let resource_rect_index = deferred_resolve.resource_address.0 as usize;
1420-
let resource_rect = &mut frame.gpu_resource_rects[resource_rect_index];
1421-
resource_rect.uv0 = Point2D::new(image.u0, image.v0);
1422-
resource_rect.uv1 = Point2D::new(image.u1, image.v1);
1423-
1424-
// If user supplied a CPU data pointer, upload it to the
1425-
// GPU now.
1426-
match image.data {
1427-
ExternalImageData::Buffer(ptr, len) => {
1428-
let data = unsafe {
1429-
slice::from_raw_parts(ptr, len)
1430-
};
1431-
1432-
// TODO(gw): This is not going to be the most efficient way to
1433-
// upload an external image on each platform. But it works for
1434-
// now and we can profile and optimize later.
1435-
self.device.update_texture(texture_id,
1436-
0,
1437-
0,
1438-
props.width,
1439-
props.height,
1440-
props.stride,
1441-
data);
1442-
1443-
// The data is uploaded to the GPU now, so client is
1444-
// able to release this image.
1445-
handler.release(external_id)
1446-
}
1447-
ExternalImageData::Native(..) => {}
1448-
}
1449-
}
1367+
self.external_images.insert(external_id, texture_id);
1368+
let resource_rect_index = deferred_resolve.resource_address.0 as usize;
1369+
let resource_rect = &mut frame.gpu_resource_rects[resource_rect_index];
1370+
resource_rect.uv0 = Point2D::new(image.u0, image.v0);
1371+
resource_rect.uv1 = Point2D::new(image.u1, image.v1);
14501372
}
14511373
}
1374+
}
1375+
1376+
fn release_external_textures(&mut self) {
1377+
if !self.external_images.is_empty() {
1378+
let handler = self.external_image_handler
1379+
.as_mut()
1380+
.expect("Found external image, but no handler set!");
1381+
1382+
for (external_id, _) in self.external_images.drain() {
1383+
handler.release(external_id);
1384+
}
1385+
}
1386+
}
1387+
1388+
fn draw_tile_frame(&mut self,
1389+
frame: &mut Frame,
1390+
framebuffer_size: &Size2D<u32>) {
1391+
self.update_deferred_resolves(frame);
14521392

14531393
// Some tests use a restricted viewport smaller than the main screen size.
14541394
// Ensure we clear the framebuffer in these tests.
@@ -1560,6 +1500,8 @@ impl Renderer {
15601500
max_prim_items,
15611501
&projection);
15621502
}
1503+
1504+
self.release_external_textures();
15631505
}
15641506

15651507
pub fn debug_renderer<'a>(&'a mut self) -> &'a mut DebugRenderer {
@@ -1575,9 +1517,10 @@ impl Renderer {
15751517
}
15761518
}
15771519

1578-
pub enum ExternalImageData {
1579-
Buffer(*const u8, usize),
1580-
Native(u32), // Is a gl::GLuint texture handle
1520+
pub enum ExternalImageSource {
1521+
// TODO(gw): Work out the API for raw buffers.
1522+
//RawData(*const u8, usize),
1523+
NativeTexture(u32), // Is a gl::GLuint texture handle
15811524
}
15821525

15831526
/// The data that an external client should provide about
@@ -1590,12 +1533,11 @@ pub enum ExternalImageData {
15901533
/// will know to re-upload the image data to the GPU.
15911534
/// Note that the UV coords are supplied in texel-space!
15921535
pub struct ExternalImage {
1593-
pub timestamp: u64,
15941536
pub u0: f32,
15951537
pub v0: f32,
15961538
pub u1: f32,
15971539
pub v1: f32,
1598-
pub data: ExternalImageData,
1540+
pub source: ExternalImageSource,
15991541
}
16001542

16011543
/// Interface that an application can implement

0 commit comments

Comments
 (0)