From 630fca300722f586b761923b88f05cbe2c471922 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 12 Oct 2012 19:51:43 -0700 Subject: [PATCH] Use LocalImageCache in LayoutTask instead of ImageCacheTask --- src/servo/css/resolve/apply.rs | 10 +-- src/servo/image/holder.rs | 85 +++++++++---------------- src/servo/layout/box_builder.rs | 3 +- src/servo/layout/context.rs | 5 +- src/servo/layout/layout_task.rs | 24 +++++-- src/servo/resource/local_image_cache.rs | 17 ++--- 6 files changed, 64 insertions(+), 80 deletions(-) diff --git a/src/servo/css/resolve/apply.rs b/src/servo/css/resolve/apply.rs index 85ef3bdddc8..ad284b9f73e 100644 --- a/src/servo/css/resolve/apply.rs +++ b/src/servo/css/resolve/apply.rs @@ -36,14 +36,12 @@ impl CSSValue : ResolveMethods { struct StyleApplicator { node: Node, - reflow: fn~(), } // TODO: normalize this into a normal preorder tree traversal function -fn apply_style(layout_ctx: &LayoutContext, node: Node, reflow: fn~()) { +fn apply_style(layout_ctx: &LayoutContext, node: Node) { let applicator = StyleApplicator { node: node, - reflow: reflow }; applicator.apply_css_style(layout_ctx); @@ -54,10 +52,9 @@ fn apply_style(layout_ctx: &LayoutContext, node: Node, reflow: fn~()) { /** A wrapper around a set of functions that can be applied as a * top-down traversal of layout boxes. */ -fn inheritance_wrapper(layout_ctx: &LayoutContext, node : Node, reflow: fn~()) { +fn inheritance_wrapper(layout_ctx: &LayoutContext, node : Node) { let applicator = StyleApplicator { node: node, - reflow: reflow }; applicator.resolve_style(layout_ctx); } @@ -104,10 +101,9 @@ fn resolve_width(box : @RenderBox) { impl StyleApplicator { fn apply_css_style(layout_ctx: &LayoutContext) { - let reflow = copy self.reflow; for NodeTree.each_child(&self.node) |child| { - inheritance_wrapper(layout_ctx, *child, copy reflow) + inheritance_wrapper(layout_ctx, *child) } } diff --git a/src/servo/image/holder.rs b/src/servo/image/holder.rs index 02d413870cf..42f860d8698 100644 --- a/src/servo/image/holder.rs +++ b/src/servo/image/holder.rs @@ -1,7 +1,9 @@ +use core::util::replace; use std::net::url::Url; use std::arc::{ARC, clone, get}; -use resource::image_cache_task::ImageCacheTask; +use resource::image_cache_task::{ImageCacheTask, ImageReady, ImageNotReady, ImageFailed}; use mod resource::image_cache_task; +use resource::local_image_cache::LocalImageCache; use geom::size::Size2D; /** A struct to store image data. The image will be loaded once, the @@ -9,24 +11,19 @@ use geom::size::Size2D; this arc are given out on demand. */ pub struct ImageHolder { - // Invariant: at least one of url and image is not none, except - // occasionally while get_image is being called - mut url : Option, + url : Url, mut image : Option>, mut cached_size: Size2D, - image_cache_task: ImageCacheTask, - reflow_cb: fn~(), - + local_image_cache: @LocalImageCache, } -fn ImageHolder(url : Url, image_cache_task: ImageCacheTask, cb: fn~()) -> ImageHolder { +fn ImageHolder(url : Url, local_image_cache: @LocalImageCache) -> ImageHolder { debug!("ImageHolder() %?", url.to_str()); let holder = ImageHolder { - url : Some(copy url), + url : url, image : None, cached_size : Size2D(0,0), - image_cache_task : image_cache_task, - reflow_cb : copy cb, + local_image_cache: local_image_cache, }; // Tell the image cache we're going to be interested in this url @@ -34,8 +31,8 @@ fn ImageHolder(url : Url, image_cache_task: ImageCacheTask, cb: fn~()) -> ImageH // but they are intended to be spread out in time. Ideally prefetch // should be done as early as possible and decode only once we // are sure that the image will be used. - image_cache_task.send(image_cache_task::Prefetch(copy url)); - image_cache_task.send(image_cache_task::Decode(move url)); + local_image_cache.prefetch(&holder.url); + local_image_cache.decode(&holder.url); holder } @@ -66,57 +63,35 @@ impl ImageHolder { } } - // This function should not be called by two tasks at the same time fn get_image() -> Option> { debug!("get_image() %?", self.url); // If this is the first time we've called this function, load // the image and store it for the future if self.image.is_none() { - let url = match copy self.url { - Some(move url) => url, - None => fail ~"expected to have a url" - }; - - let (response_chan, response_port) = pipes::stream(); - self.image_cache_task.send(image_cache_task::GetImage(copy url, response_chan)); - self.image = match response_port.recv() { - image_cache_task::ImageReady(image) => Some(clone(&image)), - image_cache_task::ImageNotReady => { - // Need to reflow when the image is available - let image_cache_task = self.image_cache_task.clone(); - let reflow = copy self.reflow_cb; - do task::spawn |copy url, move reflow| { - let (response_chan, response_port) = pipes::stream(); - image_cache_task.send(image_cache_task::WaitForImage(copy url, response_chan)); - match response_port.recv() { - image_cache_task::ImageReady(*) => reflow(), - image_cache_task::ImageNotReady => fail /*not possible*/, - image_cache_task::ImageFailed => () - } + match self.local_image_cache.get_image(&self.url).recv() { + ImageReady(move image) => { + self.image = Some(image); } - None - } - image_cache_task::ImageFailed => { - debug!("image was not ready for %s", url.to_str()); - // FIXME: Need to schedule another layout when the image is ready - None - } - }; + ImageNotReady => { + debug!("image not ready for %s", self.url.to_str()); + } + ImageFailed => { + debug!("image decoding failed for %s", self.url.to_str()); + } + } } - if self.image.is_some() { - // Temporarily swap out the arc of the image so we can clone - // it without breaking purity, then put it back and return the - // clone. This is not threadsafe. - let mut temp = None; - temp <-> self.image; - let im_arc = option::unwrap(temp); - self.image = Some(clone(&im_arc)); + // Clone isn't pure so we have to swap out the mutable image option + let image = replace(&mut self.image, None); - return Some(im_arc); - } else { - return None; - } + let result = match image { + Some(ref image) => Some(clone(image)), + None => None + }; + + replace(&mut self.image, move image); + + return result; } } diff --git a/src/servo/layout/box_builder.rs b/src/servo/layout/box_builder.rs index 1b52c55adac..dcbbe80d315 100644 --- a/src/servo/layout/box_builder.rs +++ b/src/servo/layout/box_builder.rs @@ -224,8 +224,7 @@ impl LayoutTreeBuilder { // an ICE (mozilla/rust issue #3601) if d.image.is_some() { let holder = ImageHolder({copy *d.image.get_ref()}, - layout_ctx.image_cache.clone(), - copy layout_ctx.reflow_cb); + layout_ctx.image_cache); @ImageBox(RenderBoxData(node, ctx, self.next_box_id()), holder) } else { diff --git a/src/servo/layout/context.rs b/src/servo/layout/context.rs index b1648a579ee..2a8472e2a4d 100644 --- a/src/servo/layout/context.rs +++ b/src/servo/layout/context.rs @@ -1,4 +1,4 @@ -use resource::image_cache_task::ImageCacheTask; +use resource::local_image_cache::LocalImageCache; use servo_text::font_cache::FontCache; use std::net::url::Url; use geom::rect::Rect; @@ -8,8 +8,7 @@ use au = gfx::geometry::au; struct LayoutContext { font_cache: @FontCache, - image_cache: ImageCacheTask, + image_cache: @LocalImageCache, doc_url: Url, - reflow_cb: fn~(), screen_size: Rect } \ No newline at end of file diff --git a/src/servo/layout/layout_task.rs b/src/servo/layout/layout_task.rs index e1b80154857..910b6b9f538 100644 --- a/src/servo/layout/layout_task.rs +++ b/src/servo/layout/layout_task.rs @@ -22,7 +22,8 @@ use layout::box_builder::LayoutTreeBuilder; use layout::context::LayoutContext; use opt = core::option; use render_task::RenderTask; -use resource::image_cache_task::ImageCacheTask; +use resource::image_cache_task::{ImageCacheTask, ImageResponseMsg}; +use resource::local_image_cache::LocalImageCache; use servo_text::font_cache::FontCache; use std::arc::ARC; use std::net::url::Url; @@ -60,6 +61,7 @@ fn LayoutTask(render_task: RenderTask, struct Layout { render_task: RenderTask, image_cache_task: ImageCacheTask, + local_image_cache: @LocalImageCache, from_content: comm::Port, font_cache: @FontCache, @@ -73,7 +75,8 @@ fn Layout(render_task: RenderTask, Layout { render_task: render_task, - image_cache_task: image_cache_task, + image_cache_task: image_cache_task.clone(), + local_image_cache: @LocalImageCache(move image_cache_task), from_content: from_content, font_cache: FontCache(), layout_refs: DVec() @@ -133,14 +136,16 @@ impl Layout { debug!("layout: parsed Node tree"); debug!("%?", node.dump()); + // Reset the image cache + self.local_image_cache.next_round(self.make_on_image_available_cb(to_content)); + let screen_size = Size2D(au::from_px(window_size.width as int), au::from_px(window_size.height as int)); let layout_ctx = LayoutContext { - image_cache: self.image_cache_task.clone(), + image_cache: self.local_image_cache, font_cache: self.font_cache, doc_url: doc_url, - reflow_cb: || to_content.send(ReflowEvent), screen_size: Rect(Point2D(au(0), au(0)), screen_size) }; @@ -149,7 +154,7 @@ impl Layout { node.initialize_style_for_subtree(&layout_ctx, &self.layout_refs); node.recompute_style_for_subtree(&layout_ctx, &styles); /* resolve styles (convert relative values) down the node tree */ - apply_style(&layout_ctx, node, copy layout_ctx.reflow_cb); + apply_style(&layout_ctx, node); let builder = LayoutTreeBuilder(); let layout_root: @FlowContext = match builder.construct_trees(&layout_ctx, @@ -193,5 +198,14 @@ impl Layout { true } + + // When images can't be loaded in time to display they trigger + // this callback in some task somewhere. This w + fn make_on_image_available_cb(to_content: comm::Chan) -> ~fn(ImageResponseMsg) { + let f: ~fn(ImageResponseMsg) = |_msg| { + to_content.send(ReflowEvent) + }; + return f; + } } diff --git a/src/servo/resource/local_image_cache.rs b/src/servo/resource/local_image_cache.rs index 6fbfb34e957..937a0be0a4c 100644 --- a/src/servo/resource/local_image_cache.rs +++ b/src/servo/resource/local_image_cache.rs @@ -9,27 +9,27 @@ use pipes::{Port, Chan, stream}; use image_cache_task::{ImageCacheTask, ImageResponseMsg, Prefetch, Decode, GetImage, WaitForImage}; pub fn LocalImageCache( - image_cache_task: ImageCacheTask, - on_image_available: ~fn(ImageResponseMsg) + image_cache_task: ImageCacheTask ) -> LocalImageCache { LocalImageCache { + image_cache_task: image_cache_task, round_number: 0, - image_cache_task: move image_cache_task, - on_image_available: on_image_available + mut on_image_available: None } } pub struct LocalImageCache { - priv mut round_number: uint, priv image_cache_task: ImageCacheTask, - priv on_image_available: ~fn(ImageResponseMsg) + priv mut round_number: uint, + priv mut on_image_available: Option<~fn(ImageResponseMsg)> } pub impl LocalImageCache { /// The local cache will only do a single remote request for a given /// URL in each 'round'. Layout should call this each time it begins - fn next_round() { + fn next_round(on_image_available: ~fn(ImageResponseMsg)) { self.round_number += 1; + self.on_image_available = Some(move on_image_available); } fn prefetch(url: &Url) { @@ -54,7 +54,8 @@ pub impl LocalImageCache { // the compositor should be resonsible for waiting // on the image to load and triggering layout let image_cache_task = self.image_cache_task.clone(); - let on_image_available = copy self.on_image_available; + assert self.on_image_available.is_some(); + let on_image_available = {copy *self.on_image_available.get_ref()}; let url = copy *url; do task::spawn |move url, move on_image_available| { let (response_chan, response_port) = pipes::stream();