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

Public encoding API #239

Merged
merged 4 commits into from
Jan 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion shader/fine.wgsl
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ fn fill_path(tile: Tile, xy: vec2<f32>) -> array<f32, PIXELS_PER_THREAD> {
}
// nonzero winding rule
for (var i = 0u; i < PIXELS_PER_THREAD; i += 1u) {
area[i] = abs(area[i]);
area[i] = min(abs(area[i]), 1.0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this to #241, as I feel it doesn't really belong in this PR, and I had another fine fix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

}
return area;
}
Expand Down
36 changes: 36 additions & 0 deletions src/encoding.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2022 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// Also licensed under MIT license, at your choice.

//! Raw scene encoding.

mod draw;
mod encoding;
mod math;
mod monoid;
mod packed;
mod path;

pub mod resource;

pub use draw::{
DrawBeginClip, DrawColor, DrawImage, DrawLinearGradient, DrawMonoid, DrawRadialGradient,
DrawTag,
};
pub use encoding::Encoding;
pub use math::Transform;
pub use monoid::Monoid;
pub use packed::{Config, Layout, PackedEncoding};
pub use path::{PathBbox, PathEncoder, PathMonoid, PathSegment, PathSegmentType, PathTag};
164 changes: 164 additions & 0 deletions src/encoding/draw.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
// Copyright 2022 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// Also licensed under MIT license, at your choice.

use bytemuck::{Pod, Zeroable};
use peniko::{BlendMode, Color};

use super::Monoid;

/// Draw tag representation.
#[derive(Copy, Clone, PartialEq, Eq, Pod, Zeroable)]
#[repr(C)]
pub struct DrawTag(pub u32);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it meaningful for this to be public (rather than having a getter?)

That is, is there any valid case where this could be set to a value other than one of the below constants?

I suppose it being Pod is equivalent to this being pub?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the full encoding exposed, any number of invariants can be broken so I see little value in hiding some arbitrary bits of data behind getters. For those wanting a “safe” interface, SceneBuilder should be used.


impl DrawTag {
/// No operation.
pub const NOP: Self = Self(0);

/// Color fill.
pub const COLOR: Self = Self(0x44);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The values seem very arbitrary. I see that 4 bits of the value is info_size. 6 is an awkward number of bits for that to be down, if these are being written out as (non-binary) literals.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raph can give a more informed rationale for the specific packing but various bits are used on the GPU side for computing offsets and more efficient branching on clips.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually a good point. Right now it's tightly packed rightward, but there's probably no compelling reason for that to be the case. If all our shifts were multiples of 4 then it would read as a hex value. One minor consideration is whether to count bytes or u32 words; the latter is more

Another approach to this would be to have a const fn builder that takes is_clip, scene_size (maybe "scene_words"), and info_size as args. That feels Rust-like and less magical, though unfortunately the current state of naga is that it might be clearer to use hex values as those would match CPU and GPU sides.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had considered the const fn approach as well but felt it better if these values just matched those in the shader for now. I expect these to change again when we actually handle extend modes for gradients (scene size will change) so it may be worth reconsidering the packing at 4-bit granularity then.

Copy link
Contributor

@raphlinus raphlinus Jan 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, the encoding will also change with rectangles, and it's also potentially a place to stash even/odd vs nonzero fill rules, though in the current architecture linewidth is probably a more natural fit.


/// Linear gradient fill.
pub const LINEAR_GRADIENT: Self = Self(0x114);

/// Radial gradient fill.
pub const RADIAL_GRADIENT: Self = Self(0x2dc);

/// Image fill.
pub const IMAGE: Self = Self(0x48);

/// Begin layer/clip.
pub const BEGIN_CLIP: Self = Self(0x9);

/// End layer/clip.
pub const END_CLIP: Self = Self(0x21);
}

impl DrawTag {
/// Returns the size of the info buffer used by this tag.
pub fn info_size(self) -> u32 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a candidate for being marked const fn?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

(self.0 >> 6) & 0xf
}
}

/// Draw data for a solid color.
#[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]
#[repr(C)]
pub struct DrawColor {
/// Packed RGBA color.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would probably be a good place to document the byte order.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point since it’s technically ABGR and also premultiplied.

pub rgba: u32,
}

impl DrawColor {
/// Creates new solid color draw data.
pub fn new(color: Color) -> Self {
Self {
rgba: color.to_premul_u32(),
}
}
}

/// Draw data for a linear gradient.
#[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]
#[repr(C)]
pub struct DrawLinearGradient {
/// Ramp index.
pub index: u32,
/// Start point.
pub p0: [f32; 2],
/// End point.
pub p1: [f32; 2],
}

/// Draw data for a radial gradient.
#[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]
#[repr(C)]
pub struct DrawRadialGradient {
/// Ramp index.
pub index: u32,
/// Start point.
pub p0: [f32; 2],
/// End point.
pub p1: [f32; 2],
/// Start radius.
pub r0: f32,
/// End radius.
pub r1: f32,
}

/// Draw data for an image.
#[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]
#[repr(C)]
pub struct DrawImage {
/// Image index.
pub index: u32,
/// Packed image offset.
pub offset: u32,
}

/// Draw data for a clip or layer.
#[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]
#[repr(C)]
pub struct DrawBeginClip {
/// Blend mode.
pub blend_mode: u32,
/// Group alpha.
pub alpha: f32,
}

impl DrawBeginClip {
/// Creates new clip draw data.
pub fn new(blend_mode: BlendMode, alpha: f32) -> Self {
Self {
blend_mode: (blend_mode.mix as u32) << 8 | blend_mode.compose as u32,
alpha,
}
}
}

/// Monoid for the draw tag stream.
#[derive(Copy, Clone, PartialEq, Eq, Pod, Zeroable, Default)]
#[repr(C)]
pub struct DrawMonoid {
// The number of paths preceding this draw object.
pub path_ix: u32,
// The number of clip operations preceding this draw object.
pub clip_ix: u32,
// The offset of the encoded draw object in the scene (u32s).
pub scene_offset: u32,
// The offset of the associated info.
pub info_offset: u32,
}

impl Monoid<DrawTag> for DrawMonoid {
fn new(tag: DrawTag) -> Self {
Self {
path_ix: (tag != DrawTag::NOP) as u32,
clip_ix: tag.0 & 1,
scene_offset: (tag.0 >> 2) & 0x7,
info_offset: (tag.0 >> 6) & 0xf,
}
}

fn combine(&self, other: &Self) -> Self {
Self {
path_ix: self.path_ix + other.path_ix,
clip_ix: self.clip_ix + other.clip_ix,
scene_offset: self.scene_offset + other.scene_offset,
info_offset: self.info_offset + other.info_offset,
}
}
}