From 6f9e53459ad570f365f6d1b8454c393abb52aa8f Mon Sep 17 00:00:00 2001 From: Chad Brokaw Date: Tue, 10 May 2022 20:07:41 -0400 Subject: [PATCH] Address review feedback * Change pgpu-render header file generator to add a comment noting that the file is auto-generated * Remove bin target and associated dependencies from piet-scene crate * Remove FP constructors from the Color type * Better codegen and rounding in Color::to_premul_u32() * Add kurbo attribution for piet_scene::geometry module --- Cargo.lock | 8 - pgpu-render/build.rs | 10 +- pgpu-render/pgpu.h | 2 + piet-scene/Cargo.toml | 10 - piet-scene/src/brush/color.rs | 25 +-- piet-scene/src/geometry.rs | 2 + piet-scene/src/main.rs | 356 ---------------------------------- 7 files changed, 13 insertions(+), 400 deletions(-) delete mode 100644 piet-scene/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index 938c91f..1f80fa3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1031,17 +1031,9 @@ name = "piet-scene" version = "0.1.0" dependencies = [ "bytemuck", - "clap", "moscato", - "piet", - "piet-gpu", - "piet-gpu-hal", "pinot", - "png", - "rand", - "roxmltree", "smallvec", - "winit", ] [[package]] diff --git a/pgpu-render/build.rs b/pgpu-render/build.rs index 182c72c..f565804 100644 --- a/pgpu-render/build.rs +++ b/pgpu-render/build.rs @@ -20,10 +20,10 @@ use std::env; fn main() { let crate_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); - cbindgen::Builder::new() - .with_crate(crate_dir) - .generate() - .expect("Unable to generate bindings") - .write_to_file("pgpu.h"); + .with_crate(crate_dir) + .with_header("/** Automatically generated from pgpu-render/src/lib.rs with cbindgen. **/") + .generate() + .expect("Unable to generate bindings") + .write_to_file("pgpu.h"); } diff --git a/pgpu-render/pgpu.h b/pgpu-render/pgpu.h index 93d3d5c..a72a161 100644 --- a/pgpu-render/pgpu.h +++ b/pgpu-render/pgpu.h @@ -1,3 +1,5 @@ +/** Automatically generated from pgpu-render/src/lib.rs with cbindgen. **/ + #include #include #include diff --git a/piet-scene/Cargo.toml b/piet-scene/Cargo.toml index 0e9ca25..df66483 100644 --- a/piet-scene/Cargo.toml +++ b/piet-scene/Cargo.toml @@ -9,13 +9,3 @@ bytemuck = { version = "1.7.2", features = ["derive"] } smallvec = "1.8.0" pinot = "0.1.5" moscato = "0.1.2" - -# remove these and move demo to another directory -piet-gpu = { path = "../piet-gpu" } -piet-gpu-hal = { path = "../piet-gpu-hal" } -winit = "0.25" -piet = "0.2.0" -png = "0.16.2" -rand = "0.7.3" -roxmltree = "0.13" -clap = "2.33" diff --git a/piet-scene/src/brush/color.rs b/piet-scene/src/brush/color.rs index ef519ec..a377888 100644 --- a/piet-scene/src/brush/color.rs +++ b/piet-scene/src/brush/color.rs @@ -31,28 +31,11 @@ impl Color { Self { r, g, b, a } } - pub fn rgb>(r: C, g: C, b: C) -> Self { - Self::rgb8( - (r.into() / 255.0) as u8, - (g.into() / 255.0) as u8, - (b.into() / 255.0) as u8, - ) - } - - pub fn rgba>(r: C, g: C, b: C, a: C) -> Self { - Self::rgba8( - (r.into() / 255.0) as u8, - (g.into() / 255.0) as u8, - (b.into() / 255.0) as u8, - (a.into() / 255.0) as u8, - ) - } - pub fn to_premul_u32(self) -> u32 { - let a = self.a as f64 / 255.0; - let r = (self.r as f64 * a) as u32; - let g = (self.g as f64 * a) as u32; - let b = (self.b as f64 * a) as u32; + let a = self.a as f64 * (1.0 / 255.0); + let r = (self.r as f64 * a).round() as u32; + let g = (self.g as f64 * a).round() as u32; + let b = (self.b as f64 * a).round() as u32; (r << 24) | (g << 16) | (b << 8) | self.a as u32 } } diff --git a/piet-scene/src/geometry.rs b/piet-scene/src/geometry.rs index d844013..1ea8f33 100644 --- a/piet-scene/src/geometry.rs +++ b/piet-scene/src/geometry.rs @@ -14,6 +14,8 @@ // // Also licensed under MIT license, at your choice. +// This module is based in part on kurbo (https://github.com/linebender/kurbo) + use bytemuck::{Pod, Zeroable}; use core::borrow::Borrow; use core::hash::{Hash, Hasher}; diff --git a/piet-scene/src/main.rs b/piet-scene/src/main.rs deleted file mode 100644 index f0f6e30..0000000 --- a/piet-scene/src/main.rs +++ /dev/null @@ -1,356 +0,0 @@ -use piet::kurbo::Point; -use piet::{RenderContext, Text, TextAttribute, TextLayoutBuilder}; -use piet_gpu_hal::{CmdBuf, Error, ImageLayout, Instance, Session, SubmittedCmdBuf}; - -use piet_gpu::{test_scenes, EncodedSceneRef, PietGpuRenderContext, Renderer}; - -use piet_scene::resource::ResourceContext; -use piet_scene::scene::{build_scene, Scene}; - -use clap::{App, Arg}; - -use winit::{ - event::{Event, WindowEvent}, - event_loop::{ControlFlow, EventLoop}, - window::WindowBuilder, -}; - -const NUM_FRAMES: usize = 2; - -const WIDTH: usize = 2048; -const HEIGHT: usize = 1536; - -fn main() -> Result<(), Error> { - let matches = App::new("piet-gpu test") - .arg(Arg::with_name("INPUT").index(1)) - .arg(Arg::with_name("flip").short("f").long("flip")) - .arg( - Arg::with_name("scale") - .short("s") - .long("scale") - .takes_value(true), - ) - .get_matches(); - - let event_loop = EventLoop::new(); - let window = WindowBuilder::new() - .with_inner_size(winit::dpi::LogicalSize { - width: (WIDTH / 2) as f64, - height: (HEIGHT / 2) as f64, - }) - .with_resizable(false) // currently not supported - .build(&event_loop)?; - - let (instance, surface) = Instance::new(Some(&window), Default::default())?; - let mut info_string = "info".to_string(); - - let mut scene = Scene::default(); - let mut rcx = ResourceContext::new(); - unsafe { - let device = instance.device(surface.as_ref())?; - let mut swapchain = - instance.swapchain(WIDTH / 2, HEIGHT / 2, &device, surface.as_ref().unwrap())?; - let session = Session::new(device); - - let mut current_frame = 0; - let present_semaphores = (0..NUM_FRAMES) - .map(|_| session.create_semaphore()) - .collect::, Error>>()?; - let query_pools = (0..NUM_FRAMES) - .map(|_| session.create_query_pool(12)) - .collect::, Error>>()?; - let mut cmd_bufs: [Option; NUM_FRAMES] = Default::default(); - let mut submitted: [Option; NUM_FRAMES] = Default::default(); - - let mut renderer = Renderer::new(&session, WIDTH, HEIGHT, NUM_FRAMES)?; - let mut mode = 0usize; - - event_loop.run(move |event, _, control_flow| { - *control_flow = ControlFlow::Poll; // `ControlFlow::Wait` if only re-render on event - - match event { - Event::WindowEvent { event, window_id } if window_id == window.id() => { - use winit::event::{ElementState, VirtualKeyCode}; - match event { - WindowEvent::CloseRequested => { - *control_flow = ControlFlow::Exit; - } - WindowEvent::KeyboardInput { input, .. } => { - if input.state == ElementState::Pressed { - match input.virtual_keycode { - Some(VirtualKeyCode::Left) => mode = mode.wrapping_sub(1), - Some(VirtualKeyCode::Right) => mode = mode.wrapping_add(1), - _ => {} - } - } - } - _ => (), - } - } - Event::MainEventsCleared => { - window.request_redraw(); - } - Event::RedrawRequested(window_id) if window_id == window.id() => { - let frame_idx = current_frame % NUM_FRAMES; - - if let Some(submitted) = submitted[frame_idx].take() { - cmd_bufs[frame_idx] = submitted.wait().unwrap(); - let ts = session.fetch_query_pool(&query_pools[frame_idx]).unwrap(); - if !ts.is_empty() { - info_string = format!( - "{:.3}ms :: e:{:.3}ms|alloc:{:.3}ms|cp:{:.3}ms|bd:{:.3}ms|bin:{:.3}ms|cr:{:.3}ms|r:{:.3}ms", - ts[10] * 1e3, - ts[0] * 1e3, - (ts[1] - ts[0]) * 1e3, - (ts[2] - ts[1]) * 1e3, - (ts[4] - ts[3]) * 1e3, - (ts[6] - ts[5]) * 1e3, - (ts[8] - ts[7]) * 1e3, - (ts[10] - ts[9]) * 1e3, - ); - } - } - - let mut ctx = PietGpuRenderContext::new(); - render_info_string(&mut ctx, &info_string); - - ctx = PietGpuRenderContext::new(); - test_scene1_old(&mut ctx); - //let mut encoded_scene_old = ctx.encoded_scene(); - // let ramp_data = ctx.get_ramp_data(); - //encoded_scene_old.ramp_data = &ramp_data; - test_scene1(&mut scene, &mut rcx); - let encoded_scene = scene_to_encoded_scene(&scene, &rcx); - // println!("{:?}\n============\n{:?}", encoded_scene_old, encoded_scene); - // panic!(); - let res = if mode & 1 == 0 { - render_info_string(&mut ctx, &info_string); - renderer.upload_render_ctx(&mut ctx, frame_idx) - } else { - renderer.upload_scene(&encoded_scene, frame_idx) - }; - if let Err(e) = res { - println!("error in uploading: {}", e); - } - - let (image_idx, acquisition_semaphore) = swapchain.next().unwrap(); - let swap_image = swapchain.image(image_idx); - let query_pool = &query_pools[frame_idx]; - let mut cmd_buf = cmd_bufs[frame_idx].take().unwrap_or_else(|| session.cmd_buf().unwrap()); - cmd_buf.begin(); - renderer.record(&mut cmd_buf, &query_pool, frame_idx); - - // Image -> Swapchain - cmd_buf.image_barrier( - &swap_image, - ImageLayout::Undefined, - ImageLayout::BlitDst, - ); - cmd_buf.blit_image(&renderer.image_dev, &swap_image); - cmd_buf.image_barrier(&swap_image, ImageLayout::BlitDst, ImageLayout::Present); - cmd_buf.finish(); - - submitted[frame_idx] = Some(session - .run_cmd_buf( - cmd_buf, - &[&acquisition_semaphore], - &[&present_semaphores[frame_idx]], - ) - .unwrap()); - - swapchain - .present(image_idx, &[&present_semaphores[frame_idx]]) - .unwrap(); - - current_frame += 1; - } - Event::LoopDestroyed => { - for cmd_buf in &mut submitted { - // Wait for command list submission, otherwise dropping of renderer may - // cause validation errors (and possibly crashes). - if let Some(cmd_buf) = cmd_buf.take() { - cmd_buf.wait().unwrap(); - } - } - } - _ => (), - } - }) - } -} - -fn test_scene1_old(ctx: &mut PietGpuRenderContext) { - use piet::kurbo::{Affine, Rect, Vec2}; - use piet::{Color, GradientStop}; - ctx.transform(Affine::translate(Vec2::new(200., 200.)) * Affine::rotate(45f64.to_radians())); - let linear = ctx - .gradient(piet::FixedGradient::Linear(piet::FixedLinearGradient { - start: Point::new(0., 0.), - end: Point::new(200., 100.), - stops: vec![ - GradientStop { - pos: 0.0, - color: Color::rgb8(0, 0, 255), - }, - GradientStop { - pos: 0.5, - color: Color::rgb8(0, 255, 0), - }, - GradientStop { - pos: 1.0, - color: Color::rgb8(255, 0, 0), - }, - ], - })) - .unwrap(); - let radial = ctx - .gradient(piet::FixedGradient::Radial(piet::FixedRadialGradient { - center: Point::new(50., 50.), - origin_offset: Vec2::new(0., 0.), - radius: 240., - stops: vec![ - GradientStop { - pos: 0.0, - color: Color::rgb8(0, 0, 255), - }, - GradientStop { - pos: 0.5, - color: Color::rgb8(0, 255, 0), - }, - GradientStop { - pos: 1.0, - color: Color::rgb8(255, 0, 0), - }, - ], - })) - .unwrap(); - ctx.fill_transform( - Rect { - x0: 0., - y0: 0., - x1: 200., - y1: 100., - }, - // &piet::PaintBrush::Color(piet::Color::rgb8(0, 255, 0)), - &radial, - // &piet::FixedGradient::Linear(piet::FixedLinearGradient { - // start: Point::new(0., 0.), - // end: Point::new(200., 100.), - // stops: vec![ - // GradientStop { - // pos: 0.0, - // color: Color::rgb8(0, 0, 255) - // }, - // GradientStop { - // pos: 0.5, - // color: Color::rgb8(0, 255, 0) - // }, - // GradientStop { - // pos: 1.0, - // color: Color::rgb8(255, 0, 0) - // }, - // ], - // }), - Affine::default(), // rotate(90f64.to_radians()), - ); -} - -fn test_scene1(scene: &mut Scene, rcx: &mut ResourceContext) { - use piet_scene::brush::*; - use piet_scene::geometry::{Affine, Point, Rect}; - use piet_scene::scene::*; - let mut fragment = Fragment::default(); - let mut b = build_fragment(&mut fragment); - let linear = Brush::LinearGradient(LinearGradient { - start: Point::new(0., 0.), - end: Point::new(200., 100.), - extend: Extend::Pad, - stops: (&[ - Stop { - offset: 0., - color: Color::rgb8(0, 0, 255), - }, - Stop { - offset: 0.5, - color: Color::rgb8(0, 255, 0), - }, - Stop { - offset: 1., - color: Color::rgb8(255, 0, 0), - }, - ][..]) - .into(), - }); - let radial = Brush::RadialGradient(RadialGradient { - center0: Point::new(50., 50.), - center1: Point::new(50., 50.), - radius0: 0., - radius1: 240., - extend: Extend::Pad, - stops: (&[ - Stop { - offset: 0., - color: Color::rgb8(0, 0, 255), - }, - Stop { - offset: 0.5, - color: Color::rgb8(0, 255, 0), - }, - Stop { - offset: 1., - color: Color::rgb8(255, 0, 0), - }, - ][..]) - .into(), - }); - //b.push_transform(Affine::translate(200., 200.) * Affine::rotate(45f32.to_radians())); - b.fill( - Fill::NonZero, - // &Brush::Solid(Color::rgba8(255, 0, 0, 255)), - &radial, - None, //Some(Affine::rotate(90f32.to_radians())), - Rect { - min: Point::new(0., 0.), - max: Point::new(200., 100.), - } - .elements(), - ); - b.finish(); - let mut b = build_scene(scene, rcx); - b.push_transform(Affine::translate(200., 200.) * Affine::rotate(45f32.to_radians())); - b.append(&fragment); - b.pop_transform(); - b.push_transform(Affine::translate(400., 600.)); - b.append(&fragment); - b.finish(); -} - -fn scene_to_encoded_scene<'a>( - scene: &'a Scene, - rcx: &'a ResourceContext, -) -> EncodedSceneRef<'a, piet_scene::geometry::Affine> { - let d = scene.data(); - EncodedSceneRef { - transform_stream: &d.transform_stream, - tag_stream: &d.tag_stream, - pathseg_stream: &d.pathseg_stream, - linewidth_stream: &d.linewidth_stream, - drawtag_stream: &d.drawtag_stream, - drawdata_stream: &d.drawdata_stream, - n_path: d.n_path, - n_pathseg: d.n_pathseg, - n_clip: d.n_clip, - ramp_data: rcx.ramp_data(), - } -} - -fn render_info_string(rc: &mut impl RenderContext, info: &str) { - let layout = rc - .text() - .new_text_layout(info.to_string()) - .default_attribute(TextAttribute::FontSize(40.0)) - .build() - .unwrap(); - rc.draw_text(&layout, Point::new(110.0, 50.0)); -} -