Skip to content

Commit

Permalink
Remove some cruft (#842)
Browse files Browse the repository at this point in the history
Remove outdated TODO comments.
Rewrite some other TODO comments.
Remove references to tarpaulin.

(cargo-tarpaulin is a tool used for code coverage. Masonry hasn't used
it for years, and anyway we'll probably use `#[coverage]` annotations
once they stabilize.)

---------

Co-authored-by: Daniel McNab <[email protected]>
  • Loading branch information
PoignardAzur and DJMcNab authored Jan 22, 2025
1 parent df1efa4 commit 4674f6c
Show file tree
Hide file tree
Showing 13 changed files with 19 additions and 38 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ homepage = "https://xilem.dev/"
rust.unsafe_code = "deny"

# Intentional break from the lint set. Intended to be temporary
rust.unexpected_cfgs = { level = "warn", check-cfg = ['cfg(FALSE)', 'cfg(tarpaulin_include)'] }
rust.unexpected_cfgs = { level = "warn", check-cfg = ['cfg(FALSE)'] }

# LINEBENDER LINT SET - Cargo.toml - v2
# See https://linebender.org/wiki/canonical-lints/
Expand Down
5 changes: 0 additions & 5 deletions masonry/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ use crate::kurbo::Rect;
// TODO - Occluded(bool) event
// TODO - winit ActivationTokenDone thing
// TODO - Suspended/Resume/NewEvents/MemoryWarning
// TODO - wtf is InnerSizeWriter?
// TODO - Move AnimFrame to Update
// TODO - switch anim frames to being about age / an absolute timestamp
// instead of time elapsed.
// (this will help in cases where we want to skip anim frames)

/// A global event.
#[derive(Debug, Clone)]
Expand Down
4 changes: 4 additions & 0 deletions masonry/src/passes/anim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ fn update_anim_for_widget(
);
}

// TODO - switch anim frames to being about age / an absolute timestamp
// instead of time elapsed.
// (this will help in cases where we want to skip anim frames)

/// Run the animation pass.
///
/// See the [passes documentation](../doc/05_pass_system.md#animation-pass).
Expand Down
20 changes: 5 additions & 15 deletions masonry/src/render_root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,21 +435,12 @@ impl RenderRoot {
/// Returns an update to the accessibility tree and a Vello scene representing
/// the widget tree's current state.
pub fn redraw(&mut self) -> (Scene, TreeUpdate) {
if self.root_state().needs_layout {
// TODO - Rewrite more clearly after run_rewrite_passes is rewritten
self.run_rewrite_passes();
}
if self.root_state().needs_layout {
warn!("Widget requested layout during layout pass");
self.global_state
.emit_signal(RenderRootSignal::RequestRedraw);
}
self.run_rewrite_passes();

// TODO - Handle invalidation regions
(
run_paint_pass(self),
run_accessibility_pass(self, self.scale_factor),
)
let scene = run_paint_pass(self);
let tree_update = run_accessibility_pass(self, self.scale_factor);
(scene, tree_update)
}

/// Pop the oldest signal from the queue.
Expand Down Expand Up @@ -554,7 +545,7 @@ impl RenderRoot {
/// Rewrite passes are passes which occur after external events, and
/// update flags and internal values to a consistent state.
///
/// See Pass Spec RFC for details. (TODO - Link to doc instead.)
/// See the [passes documentation](../doc/05_pass_system.md) for details.
pub(crate) fn run_rewrite_passes(&mut self) {
const REWRITE_PASSES_MAX: usize = 4;

Expand Down Expand Up @@ -597,7 +588,6 @@ impl RenderRoot {
// We request a redraw if either the render tree or the accessibility
// tree needs to be rebuilt. Usually both happen at the same time.
// A redraw will trigger a rebuild of the accessibility tree.
// TODO - We assume that a relayout will trigger a repaint
if self.root_state().needs_paint || self.root_state().needs_accessibility {
self.global_state
.emit_signal(RenderRootSignal::RequestRedraw);
Expand Down
1 change: 0 additions & 1 deletion masonry/src/testing/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,6 @@ impl TestHarness {
self.render_root.get_widget(id)
}

// TODO - Link to focus definition in tutorial
/// Return a [`WidgetRef`] to the [focused widget](crate::doc::doc_06_masonry_concepts#text-focus).
pub fn focused_widget(&self) -> Option<WidgetRef<'_, dyn Widget>> {
self.root_widget()
Expand Down
6 changes: 0 additions & 6 deletions masonry/src/testing/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,9 @@

//! Helper tools for writing unit tests.
#![cfg(not(tarpaulin_include))]

#[cfg(not(tarpaulin_include))]
mod harness;
#[cfg(not(tarpaulin_include))]
mod helper_widgets;
#[cfg(not(tarpaulin_include))]
mod screenshots;
#[cfg(not(tarpaulin_include))]
mod snapshot_utils;

pub use harness::{TestHarness, TestHarnessParams};
Expand Down
2 changes: 0 additions & 2 deletions masonry/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

//! Miscellaneous utility functions.
#![cfg(not(tarpaulin_include))]

use std::any::Any;
use std::hash::Hash;

Expand Down
2 changes: 1 addition & 1 deletion masonry/src/widget/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ mod tests {
harness.render()
};

// TODO - write comparison function
// TODO - Use Kompari instead
// We don't use assert_eq because we don't want rich assert
assert!(render_1 == render_2);
}
Expand Down
2 changes: 1 addition & 1 deletion masonry/src/widget/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2021 the Xilem Authors
// SPDX-License-Identifier: Apache-2.0

// TODO - See https://github.com/PoignardAzur/masonry-rs/issues/58
// TODO - See https://github.com/linebender/xilem/issues/336

#![allow(clippy::print_stdout, clippy::print_stderr, clippy::dbg_macro)]

Expand Down
2 changes: 1 addition & 1 deletion masonry/src/widget/tests/status_change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ fn update_hovered_on_mouse_leave() {
assert_eq!(next_hovered_changed(&button_rec), Some(false));
}

// TODO - https://github.com/PoignardAzur/masonry-rs/issues/58
// TODO - https://github.com/linebender/xilem/issues/336
#[cfg(FALSE)]
#[test]
fn update_hovered_from_layout() {
Expand Down
2 changes: 0 additions & 2 deletions masonry/src/widget/widget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ impl FromDynWidget for dyn Widget {
}
}

// TODO - Add tutorial: implementing a widget - See https://github.com/linebender/xilem/issues/376
/// The trait implemented by all widgets.
///
/// For details on how to implement this trait, see the [tutorials](crate::doc).
Expand Down Expand Up @@ -418,7 +417,6 @@ pub fn find_widget_at_pos<'c>(
/// internal implementation detail of public widgets.
pub trait AllowRawMut: Widget {}

#[cfg(not(tarpaulin_include))]
impl WidgetId {
/// Allocate a new, unique `WidgetId`.
///
Expand Down
3 changes: 2 additions & 1 deletion masonry/src/widget/widget_pod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use crate::{Affine, Widget, WidgetId};
/// Generally, container widgets don't contain other widgets directly,
/// but rather contain a `WidgetPod`, which has additional state needed
/// for layout and for the widget to participate in event flow.
// TODO - Add reference to container tutorial
///
/// See [container widget tutorial](crate::doc::doc_03_implementing_container_widget) for details.
pub struct WidgetPod<W: ?Sized> {
id: WidgetId,
inner: WidgetPodInner<W>,
Expand Down
6 changes: 4 additions & 2 deletions masonry/src/widget/widget_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ pub(crate) struct WidgetState {
/// This widget or an ancestor has been disabled.
pub(crate) is_disabled: bool,

// TODO - Document concept of "stashing".
/// This widget has been stashed.
pub(crate) is_explicitly_stashed: bool,
/// This widget or an ancestor has been stashed.
Expand All @@ -171,7 +170,10 @@ pub(crate) struct WidgetState {
pub(crate) has_focus_target: bool,

// --- DEBUG INFO ---
// TODO - document
/// The typename of the associated widget.
///
/// Used in some guard rails to provide richer error messages when a parent forgets
/// to iterate over some children.
#[cfg(debug_assertions)]
pub(crate) widget_name: &'static str,
}
Expand Down

0 comments on commit 4674f6c

Please sign in to comment.