Skip to content

/ servo

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

style: Remove Stylist::is_device_dirty. #18143

Merged
merged 3 commits into from Aug 21, 2017
+97 −108
Merged

style: Remove Stylist::is_device_dirty. #18143

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8599682
style: Remove Stylist::is_device_dirty.
emilio Aug 18, 2017
359ef7b
Don't rely on ahem being loaded sync in offset_properties_inline.html
emilio Aug 19, 2017
293274f
style: Make sure to set the initial value of the transition even if w…
emilio Aug 20, 2017
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Next

style: Remove Stylist::is_device_dirty. 

More progress on unifying how Gecko and Servo track stylist dirtiness.
  • Loading branch information
@emilio
emilio committed Aug 21, 2017
Unverified
This user has not uploaded their public key yet.
GPG key ID: 056B727BB9C1027C Learn about signing commits
commit 85996826ab7493703b1953d6784c2835347f1896
98 components/layout_thread/lib.rs
View file
Open in desktop
@@ -137,7 +137,7 @@ use style::selector_parser::SnapshotMap;
use style::servo::restyle_damage::{REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, REPOSITION, STORE_OVERFLOW};
use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, StylesheetGuards};
use style::stylesheet_set::StylesheetSet;
use style::stylesheets::{Origin, OriginSet, Stylesheet, StylesheetContents, StylesheetInDocument, UserAgentStylesheets};
use style::stylesheets::{Origin, Stylesheet, StylesheetContents, StylesheetInDocument, UserAgentStylesheets};
use style::stylist::Stylist;
use style::thread_state;
use style::timer::Timer;
@@ -434,20 +434,10 @@ impl<'a, 'b: 'a> RwData<'a, 'b> {
/// use `block`.
fn lock(&mut self) -> RWGuard<'b> {
match self.possibly_locked_rw_data.take() {
None => RWGuard::Used(self.rw_data.lock().unwrap()),
None => RWGuard::Used(self.rw_data.lock().unwrap()),
Some(x) => RWGuard::Held(x),
}
}

/// If no reflow has ever been triggered, this will keep the lock, locked
/// (and saved in `possibly_locked_rw_data`). If it has been, the lock will
/// be unlocked.
fn block(&mut self, rw_data: RWGuard<'b>) {
match rw_data {
RWGuard::Used(x) => drop(x),
RWGuard::Held(x) => *self.possibly_locked_rw_data = Some(x),
}
}
}

fn add_font_face_rules(stylesheet: &Stylesheet,
@@ -526,18 +516,6 @@ impl LayoutThread {
let font_cache_receiver =
ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_font_cache_receiver);

let stylist = Stylist::new(device, QuirksMode::NoQuirks);
let outstanding_web_fonts_counter = Arc::new(AtomicUsize::new(0));
let ua_stylesheets = &*UA_STYLESHEETS;
let guard = ua_stylesheets.shared_lock.read();
for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets {
add_font_face_rules(stylesheet,
&guard,
stylist.device(),
&font_cache_thread,
&ipc_font_cache_sender,
&outstanding_web_fonts_counter);
}

LayoutThread {
id: id,
@@ -561,7 +539,7 @@ impl LayoutThread {
generation: Cell::new(0),
new_animations_sender: new_animations_sender,
new_animations_receiver: new_animations_receiver,
outstanding_web_fonts: outstanding_web_fonts_counter,
outstanding_web_fonts: Arc::new(AtomicUsize::new(0)),
root_flow: RefCell::new(None),
document_shared_lock: None,
running_animations: ServoArc::new(RwLock::new(FnvHashMap::default())),
@@ -570,7 +548,7 @@ impl LayoutThread {
viewport_size: Size2D::new(Au(0), Au(0)),
webrender_api: webrender_api_sender.create_api(),
webrender_document,
stylist,
stylist: Stylist::new(device, QuirksMode::NoQuirks),
stylesheets: StylesheetSet::new(),
rw_data: Arc::new(Mutex::new(
LayoutThreadData {
@@ -718,12 +696,7 @@ impl LayoutThread {
match request {
Msg::AddStylesheet(stylesheet, before_stylesheet) => {
let guard = stylesheet.shared_lock.read();

self.handle_add_stylesheet(
&stylesheet,
&guard,
possibly_locked_rw_data,
);
self.handle_add_stylesheet(&stylesheet, &guard);

match before_stylesheet {
Some(insertion_point) => {
@@ -916,16 +889,13 @@ impl LayoutThread {
let _ = self.parallel_traversal.take();
}

fn handle_add_stylesheet<'a, 'b>(
fn handle_add_stylesheet(
&self,
stylesheet: &ServoArc<Stylesheet>,
stylesheet: &Stylesheet,
guard: &SharedRwLockReadGuard,
possibly_locked_rw_data: &mut RwData<'a, 'b>,
) {
// Find all font-face rules and notify the font cache of them.
// GWTODO: Need to handle unloading web fonts.

let rw_data = possibly_locked_rw_data.lock();
if stylesheet.is_effective_for_device(self.stylist.device(), &guard) {
add_font_face_rules(&*stylesheet,
&guard,
@@ -934,8 +904,6 @@ impl LayoutThread {
&self.font_cache_sender,
&self.outstanding_web_fonts);
}

possibly_locked_rw_data.block(rw_data);
}

/// Advances the animation clock of the document.
@@ -1215,7 +1183,8 @@ impl LayoutThread {
self.document_shared_lock = Some(document_shared_lock.clone());
let author_guard = document_shared_lock.read();
let device = Device::new(MediaType::screen(), initial_viewport, device_pixel_ratio);
self.stylist.set_device(device, &author_guard, self.stylesheets.iter());
let sheet_origins_affected_by_device_change =
self.stylist.set_device(device, &author_guard, self.stylesheets.iter());

self.viewport_size =
self.stylist.viewport_constraints().map_or(current_screen_size, |constraints| {
@@ -1269,20 +1238,45 @@ impl LayoutThread {
};

let needs_dirtying = {
let mut extra_data = Default::default();
let (iter, mut origins_dirty) = self.stylesheets.flush(Some(element));
debug!("Flushing stylist");

let mut origins_dirty = sheet_origins_affected_by_device_change;

debug!("Device changes: {:?}", origins_dirty);

if self.first_reflow.get() {
debug!("First reflow, rebuilding user and UA rules");

origins_dirty |= Origin::User;
origins_dirty |= Origin::UserAgent;
for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets {
self.handle_add_stylesheet(stylesheet, &ua_or_user_guard);
}
}

let (iter, invalidation_origins_dirty) = self.stylesheets.flush(Some(element));
debug!("invalidation: {:?}", invalidation_origins_dirty);

origins_dirty |= invalidation_origins_dirty;

if data.stylesheets_changed {
origins_dirty = OriginSet::all();
debug!("Doc sheets changed, flushing author sheets too");
origins_dirty |= Origin::Author;
}

if !origins_dirty.is_empty() {
let mut extra_data = Default::default();
self.stylist.rebuild(
iter,
&guards,
Some(ua_stylesheets),
/* author_style_disabled = */ false,
&mut extra_data,
origins_dirty,
);
}
let origins_rebuilt = self.stylist.rebuild(
iter,
&guards,
Some(ua_stylesheets),
/* author_style_disabled = */ false,
&mut extra_data,
origins_dirty,
);
!origins_rebuilt.is_empty()

!origins_dirty.is_empty()
};

let needs_reflow = viewport_size_changed && !needs_dirtying;
83 components/style/stylist.rs
View file
Open in desktop
@@ -78,9 +78,6 @@ pub struct Stylist {
#[cfg_attr(feature = "servo", ignore_heap_size_of = "defined in selectors")]
quirks_mode: QuirksMode,

/// If true, the device has changed, and the stylist needs to be updated.
is_device_dirty: bool,

/// Selector maps for all of the style sheets in the stylist, after
/// evalutaing media rules against the current device, split out per
/// cascade level.
@@ -130,7 +127,6 @@ impl Stylist {
Stylist {
viewport_constraints: None,
device: device,
is_device_dirty: true,
quirks_mode: quirks_mode,

cascade_data: Default::default(),
@@ -184,30 +180,20 @@ impl Stylist {

/// Rebuild the stylist for the given document stylesheets, and optionally
/// with a set of user agent stylesheets.
///
/// This method resets all the style data each time the stylesheets change
/// (which is indicated by the `stylesheets_changed` parameter), or the
/// device is dirty, which means we need to re-evaluate media queries.
pub fn rebuild<'a, I, S>(
&mut self,
doc_stylesheets: I,
guards: &StylesheetGuards,
ua_stylesheets: Option<&UserAgentStylesheets>,
author_style_disabled: bool,
extra_data: &mut PerOrigin<ExtraStyleData>,
mut origins_to_rebuild: OriginSet,
) -> OriginSet
origins_to_rebuild: OriginSet,
)
where
I: Iterator<Item = &'a S> + Clone,
S: StylesheetInDocument + ToMediaListKey + 'static,
{
if self.is_device_dirty {
origins_to_rebuild = OriginSet::all();
}

if origins_to_rebuild.is_empty() {
return origins_to_rebuild;
}
debug_assert!(!origins_to_rebuild.is_empty());

self.num_rebuilds += 1;

@@ -252,19 +238,40 @@ impl Stylist {

if let Some(ua_stylesheets) = ua_stylesheets {
for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets {
let sheet_origin =
stylesheet.contents(guards.ua_or_user).origin;

debug_assert!(matches!(
stylesheet.contents(guards.ua_or_user).origin,
Origin::UserAgent | Origin::User));
self.add_stylesheet(stylesheet, guards.ua_or_user, extra_data);
sheet_origin,
Origin::UserAgent | Origin::User
));

if origins_to_rebuild.contains(sheet_origin.into()) {
self.add_stylesheet(
stylesheet,
guards.ua_or_user,
extra_data
);
}
}

if self.quirks_mode != QuirksMode::NoQuirks {
let stylesheet = &ua_stylesheets.quirks_mode_stylesheet;
let sheet_origin =
stylesheet.contents(guards.ua_or_user).origin;

debug_assert!(matches!(
stylesheet.contents(guards.ua_or_user).origin,
Origin::UserAgent | Origin::User));
self.add_stylesheet(&ua_stylesheets.quirks_mode_stylesheet,
guards.ua_or_user, extra_data);
sheet_origin,
Origin::UserAgent | Origin::User
));

if origins_to_rebuild.contains(sheet_origin.into()) {
self.add_stylesheet(
&ua_stylesheets.quirks_mode_stylesheet,
guards.ua_or_user,
extra_data
);
}
}
}

@@ -280,9 +287,6 @@ impl Stylist {
for stylesheet in sheets_to_add {
self.add_stylesheet(stylesheet, guards.author, extra_data);
}

self.is_device_dirty = false;
origins_to_rebuild
}

fn add_stylesheet<S>(
@@ -831,18 +835,14 @@ impl Stylist {
/// Set a given device, which may change the styles that apply to the
/// document.
///
/// Returns the sheet origins that were actually affected.
///
/// This means that we may need to rebuild style data even if the
/// stylesheets haven't changed.
///
/// Also, the device that arrives here may need to take the viewport rules
/// into account.
///
/// TODO(emilio): Probably should be unified with `update`, right now I
/// don't think we take into account dynamic updates to viewport rules.
///
/// Probably worth to make the stylist own a single `Device`, and have a
/// `update_device` function?
///
/// feature = "servo" because gecko only has one device, and manually tracks
/// when the device is dirty.
///
@@ -854,7 +854,7 @@ impl Stylist {
mut device: Device,
guard: &SharedRwLockReadGuard,
stylesheets: I,
)
) -> OriginSet
where
I: Iterator<Item = &'a S> + Clone,
S: StylesheetInDocument + ToMediaListKey + 'static,
@@ -875,11 +875,10 @@ impl Stylist {
}

self.device = device;
let features_changed = self.media_features_change_changed_style(
self.media_features_change_changed_style(
stylesheets,
guard
);
self.is_device_dirty |= !features_changed.is_empty();
guard,
)
}

/// Returns whether, given a media feature change, any previously-applicable
@@ -1067,7 +1066,6 @@ impl Stylist {
V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock> + Debug,
F: FnMut(&E, ElementSelectorFlags),
{
debug_assert!(!self.is_device_dirty);
// Gecko definitely has pseudo-elements with style attributes, like
// ::-moz-color-swatch.
debug_assert!(cfg!(feature = "gecko") ||
@@ -1217,13 +1215,6 @@ impl Stylist {
.any(|(d, _)| d.mapped_ids.might_contain_hash(id.get_hash()))
}

/// Return whether the device is dirty, that is, whether the screen size or
/// media type have changed (for now).
#[inline]
pub fn is_device_dirty(&self) -> bool {
self.is_device_dirty
}

/// Returns the registered `@keyframes` animation for the specified name.
#[inline]
pub fn get_animation(&self, name: &Atom) -> Option<&KeyframesAnimation> {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.