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

Remove redundant cluster range, as an optimization. #3433

Merged
merged 1 commit into from Dec 18, 2018
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -31,7 +31,7 @@ use scene::{FilterOpHelpers, SceneProperties};
use scene_builder::DocumentResources;
use smallvec::SmallVec;
use surface::{SurfaceDescriptor, TransformKey};
use std::{mem, ops};
use std::{mem, u16};
use texture_cache::{Eviction, TextureCacheHandle};
use tiling::RenderTargetKind;
use util::{TransformedRectKind, MatrixHelpers, MaxRect, RectHelpers};
@@ -710,25 +710,7 @@ impl TileCache {
// been marked invisible, we exclude it here. Otherwise, we may end up
// with a primitive that is outside the bounding rect of the calculated
// picture rect (which takes the cluster visibility into account).
// TODO(gw): It turns out that we have ended up having only a single
// cluster per primitive rather than a range. In future,
// we should tidy this up to take advantage of this!
let mut in_visible_cluster = false;
for ci in prim_instance.cluster_range.start .. prim_instance.cluster_range.end {
// Map from the cluster range index to a cluster index
let cluster_index = prim_list.prim_cluster_map[ci as usize];

// Get the cluster and see if is visible
let cluster = &prim_list.clusters[cluster_index.0 as usize];
in_visible_cluster |= cluster.is_visible;

// As soon as a primitive is in a visible cluster, it's considered
// visible and we don't need to consult other clusters.
if cluster.is_visible {
break;
}
}
if !in_visible_cluster {
if !prim_list.clusters[prim_instance.cluster_index.0 as usize].is_visible {
return;
}

@@ -1344,7 +1326,12 @@ impl PrimitiveCluster {
#[derive(Debug, Copy, Clone)]
pub struct PrimitiveClusterIndex(pub u32);

pub type ClusterRange = ops::Range<u32>;
#[derive(Debug, Copy, Clone)]
pub struct ClusterIndex(pub u16);

impl ClusterIndex {
pub const INVALID: ClusterIndex = ClusterIndex(u16::MAX);
}

/// A list of pictures, stored by the PrimitiveList to enable a
/// fast traversal of just the pictures.
@@ -1362,10 +1349,6 @@ pub struct PrimitiveList {
pub pictures: PictureList,
/// List of primitives grouped into clusters.
pub clusters: SmallVec<[PrimitiveCluster; 4]>,
/// This maps from the cluster_range in a primitive
/// instance to a set of cluster(s) that the
/// primitive instance belongs to.
pub prim_cluster_map: Vec<PrimitiveClusterIndex>,
}

impl PrimitiveList {
@@ -1378,7 +1361,6 @@ impl PrimitiveList {
prim_instances: Vec::new(),
pictures: SmallVec::new(),
clusters: SmallVec::new(),
prim_cluster_map: Vec::new(),
}
}

@@ -1393,7 +1375,6 @@ impl PrimitiveList {
let mut pictures = SmallVec::new();
let mut clusters_map = FastHashMap::default();
let mut clusters: SmallVec<[PrimitiveCluster; 4]> = SmallVec::new();
let mut prim_cluster_map = Vec::new();

// Walk the list of primitive instances and extract any that
// are pictures.
@@ -1479,27 +1460,13 @@ impl PrimitiveList {
cluster.bounding_rect = cluster.bounding_rect.union(&culling_rect);
}

// Define a range of clusters that this primitive belongs to. For now, this
// seems like overkill, since a primitive only ever belongs to one cluster.
// However, in the future the clusters will include spatial information. It
// will often be the case that a primitive may overlap more than one cluster,
// and belong to several.
let start = prim_cluster_map.len() as u32;
let cluster_range = ClusterRange {
start,
end: start + 1,
};

// Store the cluster index in the map, and the range in the instance.
prim_cluster_map.push(PrimitiveClusterIndex(cluster_index as u32));
prim_instance.cluster_range = cluster_range;
prim_instance.cluster_index = ClusterIndex(cluster_index as u16);
}

PrimitiveList {
prim_instances,
pictures,
clusters,
prim_cluster_map,
}
}
}
@@ -26,7 +26,7 @@ use gpu_types::BrushFlags;
use image::{Repetition};
use intern;
use picture::{PictureCompositeMode, PicturePrimitive, PictureUpdateState, TileCacheUpdateState};
use picture::{ClusterRange, PrimitiveList, SurfaceIndex, SurfaceInfo, RetainedTiles, RasterConfig};
use picture::{ClusterIndex, PrimitiveList, SurfaceIndex, SurfaceInfo, RetainedTiles, RasterConfig};
use prim_store::borders::{ImageBorderDataHandle, NormalBorderDataHandle};
use prim_store::gradient::{LinearGradientDataHandle, RadialGradientDataHandle};
use prim_store::image::{ImageDataHandle, ImageInstance, VisibleImageTile, YuvImageDataHandle};
@@ -309,7 +309,7 @@ pub struct DeferredResolve {
}

#[derive(Debug, Copy, Clone, PartialEq)]
pub struct ClipTaskIndex(pub u32);
pub struct ClipTaskIndex(pub u16);

impl ClipTaskIndex {
pub const INVALID: ClipTaskIndex = ClipTaskIndex(0);
@@ -1558,14 +1558,16 @@ pub struct PrimitiveInstance {
/// a list of clip task ids (one per segment).
pub clip_task_index: ClipTaskIndex,

/// The cluster that this primitive belongs to. This is used
/// for quickly culling out groups of primitives during the
/// initial picture traversal pass.
pub cluster_index: ClusterIndex,

/// ID of the clip chain that this primitive is clipped by.
pub clip_chain_id: ClipChainId,

/// ID of the spatial node that this primitive is positioned by.
pub spatial_node_index: SpatialNodeIndex,

/// A range of clusters that this primitive instance belongs to.
pub cluster_range: ClusterRange,
}

impl PrimitiveInstance {
@@ -1587,7 +1589,7 @@ impl PrimitiveInstance {
clip_task_index: ClipTaskIndex::INVALID,
clip_chain_id,
spatial_node_index,
cluster_range: ClusterRange { start: 0, end: 0 },
cluster_index: ClusterIndex::INVALID,
}
}

@@ -2392,40 +2394,8 @@ impl PrimitiveStore {
prim_instance.id, pic_context.pipeline_id);
}

// Run through the list of cluster(s) this primitive belongs
// to. As soon as we find one visible cluster that this
// primitive belongs to, then the primitive itself can be
// considered visible.
// TODO(gw): Initially, primitive clusters are only used
// to group primitives by backface visibility and
// whether a spatial node is invertible or not.
// In the near future, clusters will also act as
// a simple spatial hash for grouping.
// TODO(gw): For now, walk the primitive list and check if
// it is visible in any clusters, as this is a
// simple way to retain correct render order. In
// the future, it might make sense to invert this
// and have the cluster visibility pass produce
// an index buffer / set of primitive instances
// that we sort into render order.
let mut in_visible_cluster = false;
for ci in prim_instance.cluster_range.start .. prim_instance.cluster_range.end {
// Map from the cluster range index to a cluster index
let cluster_index = prim_list.prim_cluster_map[ci as usize];

// Get the cluster and see if is visible
let cluster = &prim_list.clusters[cluster_index.0 as usize];
in_visible_cluster |= cluster.is_visible;

// As soon as a primitive is in a visible cluster, it's considered
// visible and we don't need to consult other clusters.
if cluster.is_visible {
break;
}
}

// If the primitive wasn't in any visible clusters, it can be skipped.
if !in_visible_cluster {
// Get the cluster and see if is visible
if !prim_list.clusters[prim_instance.cluster_index.0 as usize].is_visible {
continue;
}

@@ -3556,7 +3526,7 @@ fn test_struct_sizes() {
// test expectations and move on.
// (b) You made a structure larger. This is not necessarily a problem, but should only
// be done with care, and after checking if talos performance regresses badly.
assert_eq!(mem::size_of::<PrimitiveInstance>(), 128, "PrimitiveInstance size changed");
assert_eq!(mem::size_of::<PrimitiveInstance>(), 120, "PrimitiveInstance size changed");
assert_eq!(mem::size_of::<PrimitiveInstanceKind>(), 40, "PrimitiveInstanceKind size changed");
assert_eq!(mem::size_of::<PrimitiveTemplate>(), 96, "PrimitiveTemplate size changed");
assert_eq!(mem::size_of::<PrimitiveTemplateKind>(), 36, "PrimitiveTemplateKind size changed");
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.