Checkpoint

Nathan Sobo created

Change summary

crates/gpui/src/platform/mac/atlas.rs           |  5 
crates/gpui/src/platform/mac/renderer.rs        |  3 
crates/gpui/src/platform/mac/sprite_cache.rs    |  2 
crates/gpui3/src/app/entity_map.rs              | 11 +-
crates/gpui3/src/platform.rs                    | 16 +--
crates/gpui3/src/platform/mac/metal_atlas.rs    | 13 +-
crates/gpui3/src/platform/mac/metal_renderer.rs | 71 +++++++++++++-----
crates/gpui3/src/platform/mac/text_system.rs    | 24 ++----
crates/gpui3/src/platform/mac/window.rs         |  8 +-
crates/gpui3/src/text_system.rs                 | 10 +-
crates/gpui3/src/window.rs                      | 13 +--
11 files changed, 96 insertions(+), 80 deletions(-)

Detailed changes

crates/gpui/src/platform/mac/atlas.rs 🔗

@@ -109,6 +109,7 @@ impl AtlasAllocator {
             };
             descriptor.set_width(size.x() as u64);
             descriptor.set_height(size.y() as u64);
+
             self.device.new_texture(&descriptor)
         } else {
             self.device.new_texture(&self.texture_descriptor)
@@ -146,10 +147,10 @@ impl Atlas {
             bounds.size().y() as u64,
         );
         self.texture.replace_region(
-            region,
+            dbg!(region),
             0,
             bytes.as_ptr() as *const _,
-            (bounds.size().x() * self.bytes_per_pixel() as i32) as u64,
+            dbg!((bounds.size().x() * self.bytes_per_pixel() as i32) as u64),
         );
     }
 

crates/gpui/src/platform/mac/renderer.rs 🔗

@@ -632,7 +632,8 @@ impl Renderer {
             ) {
                 // Snap sprite to pixel grid.
                 let origin = (glyph.origin * scale_factor).floor() + sprite.offset.to_f32();
-                // dbg!(origin);
+
+                dbg!(origin);
                 sprites_by_atlas
                     .entry(sprite.atlas_id)
                     .or_insert_with(Vec::new)

crates/gpui/src/platform/mac/sprite_cache.rs 🔗

@@ -114,7 +114,7 @@ impl SpriteCache {
 
                 let (alloc_id, atlas_bounds) = self
                     .atlases
-                    .upload(dbg!(glyph_bounds.size()), &mask)
+                    .upload(glyph_bounds.size(), &mask)
                     .expect("could not upload glyph");
                 Some(GlyphSprite {
                     atlas_id: alloc_id.atlas_id,

crates/gpui3/src/app/entity_map.rs 🔗

@@ -125,11 +125,12 @@ impl<T: Send + Sync> Clone for Handle<T> {
 
 impl<T: Send + Sync> Drop for Handle<T> {
     fn drop(&mut self) {
-        if let Some(ref_counts) = self.ref_counts.upgrade() {
-            if let Some(count) = ref_counts.read().get(self.id) {
-                let prev_count = count.fetch_sub(1, SeqCst);
-                assert_ne!(prev_count, 0, "Detected over-release of a handle.");
-            }
+        if let Some(_ref_counts) = self.ref_counts.upgrade() {
+            // todo!()
+            // if let Some(count) = ref_counts.read().get(self.id) {
+            //     let prev_count = count.fetch_sub(1, SeqCst);
+            //     assert_ne!(prev_count, 0, "Detected over-release of a handle.");
+            // }
         }
     }
 }

crates/gpui3/src/platform.rs 🔗

@@ -6,8 +6,8 @@ mod mac;
 mod test;
 
 use crate::{
-    AnyWindowHandle, Bounds, DevicePixels, Font, FontId, FontMetrics, GlyphId,
-    GlyphRasterizationParams, Pixels, Point, Result, Scene, ShapedLine, SharedString, Size,
+    AnyWindowHandle, Bounds, DevicePixels, Font, FontId, FontMetrics, GlyphId, GlyphRasterParams,
+    Pixels, Point, Result, Scene, ShapedLine, SharedString, Size,
 };
 use anyhow::anyhow;
 use async_task::Runnable;
@@ -147,7 +147,7 @@ pub trait PlatformWindow {
     fn is_topmost_for_position(&self, position: Point<Pixels>) -> bool;
     fn draw(&self, scene: Scene);
 
-    fn glyph_atlas(&self) -> Arc<dyn PlatformAtlas<GlyphRasterizationParams>>;
+    fn glyph_atlas(&self) -> Arc<dyn PlatformAtlas<GlyphRasterParams>>;
 }
 
 pub trait PlatformDispatcher: Send + Sync {
@@ -163,14 +163,8 @@ pub trait PlatformTextSystem: Send + Sync {
     fn typographic_bounds(&self, font_id: FontId, glyph_id: GlyphId) -> Result<Bounds<f32>>;
     fn advance(&self, font_id: FontId, glyph_id: GlyphId) -> Result<Size<f32>>;
     fn glyph_for_char(&self, font_id: FontId, ch: char) -> Option<GlyphId>;
-    fn glyph_raster_bounds(
-        &self,
-        params: &GlyphRasterizationParams,
-    ) -> Result<Bounds<DevicePixels>>;
-    fn rasterize_glyph(
-        &self,
-        params: &GlyphRasterizationParams,
-    ) -> Result<(Size<DevicePixels>, Vec<u8>)>;
+    fn glyph_raster_bounds(&self, params: &GlyphRasterParams) -> Result<Bounds<DevicePixels>>;
+    fn rasterize_glyph(&self, params: &GlyphRasterParams) -> Result<(Size<DevicePixels>, Vec<u8>)>;
     fn layout_line(&self, text: &str, font_size: Pixels, runs: &[(usize, FontId)]) -> ShapedLine;
     fn wrap_line(
         &self,

crates/gpui3/src/platform/mac/metal_atlas.rs 🔗

@@ -93,7 +93,7 @@ impl<Key> MetalAtlasState<Key> {
             };
             descriptor.set_width(min_size.width.into());
             descriptor.set_height(min_size.height.into());
-
+            descriptor.set_pixel_format(metal::MTLPixelFormat::Depth32Float);
             size = min_size;
             metal_texture = self.device.new_texture(&descriptor);
         } else {
@@ -119,17 +119,16 @@ struct MetalAtlasTexture {
 
 impl MetalAtlasTexture {
     fn upload(&mut self, size: Size<DevicePixels>, bytes: &[u8]) -> Option<AtlasTile> {
-        dbg!(size);
-        let size = size.into();
-        let allocation = self.allocator.allocate(size)?;
+        let allocation = self.allocator.allocate(size.into())?;
         let tile = AtlasTile {
             texture_id: self.id,
             tile_id: allocation.id.into(),
-            bounds: allocation.rectangle.into(),
+            bounds: Bounds {
+                origin: allocation.rectangle.min.into(),
+                size,
+            },
         };
 
-        // eprintln!("upload {:?}", tile.bounds);
-
         let region = metal::MTLRegion::new_2d(
             tile.bounds.origin.x.into(),
             tile.bounds.origin.y.into(),

crates/gpui3/src/platform/mac/metal_renderer.rs 🔗

@@ -1,6 +1,40 @@
+// How can I fix this?
+// -[MTLDebugRenderCommandEncoder setRenderPipelineState:]:1580: failed assertion `Set Render Pipeline State Validation
+// For depth attachment, the render pipeline's pixelFormat (MTLPixelFormatInvalid) does not match the framebuffer's pixelFormat (MTLPixelFormatDepth32Float).
+// '
+// -[MTLDebugRenderCommandEncoder setRenderPipelineState:]:1580: failed assertion `Set Render Pipeline State Validation
+// For depth attachment, the render pipeline's pixelFormat (MTLPixelFormatInvalid) does not match the framebuffer's pixelFormat (MTLPixelFormatDepth32Float).
+// // It seems like the error you're facing has to do with the difference between the
+// pixel format of the render pipeline and the framebuffer. If the pixel format of
+// those two doesn't match, Metal throws an error. To resolve this issue, you need
+// to set the pixel format of your depth attachment and your render pipeline state
+// to the same value.
+
+// In this code:
+// ---
+/*
+descriptor.set_depth_attachment_pixel_format(MTLPixelFormat::Depth32Float);
+*/
+// ---
+// you've commented out the line where you set the depth attachment pixel format
+// to MTLPixelFormat::Depth32Float. If you uncomment this line, it should resolve
+// the error as your depth attachment's pixel format will then match your framebuffer's.
+
+// If you still encounter the same problem, you might be configuring another render
+// pipeline state elsewhere in your code with a different depth pixel format. Make
+// sure all configurations have matching pixel formats.
+
+// Additionally, be aware of the limitations of certain pixel formats. For example,
+// not all pixel formats support depth stencil attachments, and some are only
+// compatible with certain types of GPU hardware. Implementation of pixel formats
+// can vary between different versions of iOS, so ensure that your choice of pixel
+// format is compatible with your minimum target version.
+//
+// I want it to be UANorm
+
 use crate::{
-    point, size, AtlasTextureId, DevicePixels, GlyphRasterizationParams, MetalAtlas,
-    MonochromeSprite, Quad, Scene, Size,
+    point, size, AtlasTextureId, DevicePixels, GlyphRasterParams, MetalAtlas, MonochromeSprite,
+    Quad, Scene, Size,
 };
 use cocoa::{
     base::{NO, YES},
@@ -22,7 +56,7 @@ pub struct MetalRenderer {
     sprites_pipeline_state: metal::RenderPipelineState,
     unit_vertices: metal::Buffer,
     instances: metal::Buffer,
-    glyph_atlas: Arc<MetalAtlas<GlyphRasterizationParams>>,
+    glyph_atlas: Arc<MetalAtlas<GlyphRasterParams>>,
 }
 
 impl MetalRenderer {
@@ -104,7 +138,7 @@ impl MetalRenderer {
         let glyph_atlas = Arc::new(MetalAtlas::new(
             Size {
                 width: DevicePixels(1024),
-                height: DevicePixels(1024),
+                height: DevicePixels(768),
             },
             MTLPixelFormat::A8Unorm,
             device.clone(),
@@ -126,7 +160,7 @@ impl MetalRenderer {
         &*self.layer
     }
 
-    pub fn glyph_atlas(&self) -> &Arc<MetalAtlas<GlyphRasterizationParams>> {
+    pub fn glyph_atlas(&self) -> &Arc<MetalAtlas<GlyphRasterParams>> {
         &self.glyph_atlas
     }
 
@@ -151,18 +185,17 @@ impl MetalRenderer {
 
         let render_pass_descriptor = metal::RenderPassDescriptor::new();
 
-        let depth_texture_desc = metal::TextureDescriptor::new();
-        depth_texture_desc.set_pixel_format(metal::MTLPixelFormat::Depth32Float);
-        depth_texture_desc.set_storage_mode(metal::MTLStorageMode::Private);
-        depth_texture_desc.set_usage(metal::MTLTextureUsage::RenderTarget);
-        depth_texture_desc.set_width(i32::from(viewport_size.width) as u64);
-        depth_texture_desc.set_height(i32::from(viewport_size.height) as u64);
-        let depth_texture = self.device.new_texture(&depth_texture_desc);
-        let depth_attachment = render_pass_descriptor.depth_attachment().unwrap();
-
-        depth_attachment.set_texture(Some(&depth_texture));
-        depth_attachment.set_clear_depth(1.);
-        depth_attachment.set_store_action(metal::MTLStoreAction::Store);
+        // let depth_texture_desc = metal::TextureDescriptor::new();
+        // depth_texture_desc.set_pixel_format(metal::MTLPixelFormat::Depth32Float);
+        // depth_texture_desc.set_storage_mode(metal::MTLStorageMode::Private);
+        // depth_texture_desc.set_usage(metal::MTLTextureUsage::RenderTarget);
+        // depth_texture_desc.set_width(i32::from(viewport_size.width) as u64);
+        // depth_texture_desc.set_height(i32::from(viewport_size.height) as u64);
+        // let depth_texture = self.device.new_texture(&depth_texture_desc);
+        // let depth_attachment = render_pass_descriptor.depth_attachment().unwrap();
+        // depth_attachment.set_texture(Some(&depth_texture));
+        // depth_attachment.set_clear_depth(1.);
+        // depth_attachment.set_store_action(metal::MTLStoreAction::Store);
 
         let color_attachment = render_pass_descriptor
             .color_attachments()
@@ -289,8 +322,6 @@ impl MetalRenderer {
         viewport_size: Size<DevicePixels>,
         command_encoder: &metal::RenderCommandEncoderRef,
     ) {
-        // dbg!(sprites);
-
         if sprites.is_empty() {
             return;
         }
@@ -386,7 +417,7 @@ fn build_pipeline_state(
     color_attachment.set_source_alpha_blend_factor(metal::MTLBlendFactor::One);
     color_attachment.set_destination_rgb_blend_factor(metal::MTLBlendFactor::OneMinusSourceAlpha);
     color_attachment.set_destination_alpha_blend_factor(metal::MTLBlendFactor::One);
-    // descriptor.set_depth_attachment_pixel_format(MTLPixelFormat::Depth32Float);
+    descriptor.set_depth_attachment_pixel_format(MTLPixelFormat::Invalid);
 
     device
         .new_render_pipeline_state(&descriptor)

crates/gpui3/src/platform/mac/text_system.rs 🔗

@@ -1,7 +1,7 @@
 use crate::{
     point, px, size, Bounds, DevicePixels, Font, FontFeatures, FontId, FontMetrics, FontStyle,
-    FontWeight, GlyphId, GlyphRasterizationParams, Pixels, PlatformTextSystem, Point, Result,
-    ShapedGlyph, ShapedLine, ShapedRun, SharedString, Size, SUBPIXEL_VARIANTS,
+    FontWeight, GlyphId, GlyphRasterParams, Pixels, PlatformTextSystem, Point, Result, ShapedGlyph,
+    ShapedLine, ShapedRun, SharedString, Size, SUBPIXEL_VARIANTS,
 };
 use anyhow::anyhow;
 use cocoa::appkit::{CGFloat, CGPoint};
@@ -134,16 +134,13 @@ impl PlatformTextSystem for MacTextSystem {
         self.0.read().glyph_for_char(font_id, ch)
     }
 
-    fn glyph_raster_bounds(
-        &self,
-        params: &GlyphRasterizationParams,
-    ) -> Result<Bounds<DevicePixels>> {
+    fn glyph_raster_bounds(&self, params: &GlyphRasterParams) -> Result<Bounds<DevicePixels>> {
         self.0.read().raster_bounds(params)
     }
 
     fn rasterize_glyph(
         &self,
-        glyph_id: &GlyphRasterizationParams,
+        glyph_id: &GlyphRasterParams,
     ) -> Result<(Size<DevicePixels>, Vec<u8>)> {
         self.0.read().rasterize_glyph(glyph_id)
     }
@@ -237,7 +234,7 @@ impl MacTextSystemState {
             })
     }
 
-    fn raster_bounds(&self, params: &GlyphRasterizationParams) -> Result<Bounds<DevicePixels>> {
+    fn raster_bounds(&self, params: &GlyphRasterParams) -> Result<Bounds<DevicePixels>> {
         let font = &self.fonts[params.font_id.0];
         let scale = Transform2F::from_scale(params.scale_factor);
         Ok(font
@@ -251,10 +248,7 @@ impl MacTextSystemState {
             .into())
     }
 
-    fn rasterize_glyph(
-        &self,
-        params: &GlyphRasterizationParams,
-    ) -> Result<(Size<DevicePixels>, Vec<u8>)> {
+    fn rasterize_glyph(&self, params: &GlyphRasterParams) -> Result<(Size<DevicePixels>, Vec<u8>)> {
         let glyph_bounds = self.raster_bounds(params)?;
         if glyph_bounds.size.width.0 == 0 || glyph_bounds.size.height.0 == 0 {
             Err(anyhow!("glyph bounds are empty"))
@@ -292,7 +286,7 @@ impl MacTextSystemState {
 
             let subpixel_shift = params
                 .subpixel_variant
-                .map(|v| v as f32 / SUBPIXEL_VARIANTS as f32 / params.scale_factor);
+                .map(|v| v as f32 / SUBPIXEL_VARIANTS as f32);
             cx.set_allows_font_subpixel_positioning(true);
             cx.set_should_subpixel_position_fonts(true);
             cx.set_allows_font_subpixel_quantization(false);
@@ -303,8 +297,8 @@ impl MacTextSystemState {
                 .draw_glyphs(
                     &[u32::from(params.glyph_id) as CGGlyph],
                     &[CGPoint::new(
-                        subpixel_shift.x as CGFloat,
-                        subpixel_shift.y as CGFloat,
+                        (subpixel_shift.x / params.scale_factor) as CGFloat,
+                        (subpixel_shift.y / params.scale_factor) as CGFloat,
                     )],
                     cx,
                 );

crates/gpui3/src/platform/mac/window.rs 🔗

@@ -1,8 +1,8 @@
 use super::{ns_string, MetalRenderer, NSRange};
 use crate::{
-    point, px, size, AnyWindowHandle, Bounds, Event, GlyphRasterizationParams, KeyDownEvent,
-    Keystroke, MacScreen, Modifiers, ModifiersChangedEvent, MouseButton, MouseDownEvent,
-    MouseMovedEvent, MouseUpEvent, NSRectExt, Pixels, Platform, PlatformAtlas, PlatformDispatcher,
+    point, px, size, AnyWindowHandle, Bounds, Event, GlyphRasterParams, KeyDownEvent, Keystroke,
+    MacScreen, Modifiers, ModifiersChangedEvent, MouseButton, MouseDownEvent, MouseMovedEvent,
+    MouseUpEvent, NSRectExt, Pixels, Platform, PlatformAtlas, PlatformDispatcher,
     PlatformInputHandler, PlatformScreen, PlatformWindow, Point, Scene, Size, Timer,
     WindowAppearance, WindowBounds, WindowKind, WindowOptions, WindowPromptLevel,
 };
@@ -886,7 +886,7 @@ impl PlatformWindow for MacWindow {
         }
     }
 
-    fn glyph_atlas(&self) -> Arc<dyn PlatformAtlas<GlyphRasterizationParams>> {
+    fn glyph_atlas(&self) -> Arc<dyn PlatformAtlas<GlyphRasterParams>> {
         self.0.lock().renderer.glyph_atlas().clone()
     }
 }

crates/gpui3/src/text_system.rs 🔗

@@ -215,13 +215,13 @@ impl TextSystem {
         })
     }
 
-    pub fn raster_bounds(&self, params: &GlyphRasterizationParams) -> Result<Bounds<DevicePixels>> {
+    pub fn raster_bounds(&self, params: &GlyphRasterParams) -> Result<Bounds<DevicePixels>> {
         self.platform_text_system.glyph_raster_bounds(params)
     }
 
     pub fn rasterize_glyph(
         &self,
-        glyph_id: &GlyphRasterizationParams,
+        glyph_id: &GlyphRasterParams,
     ) -> Result<(Size<DevicePixels>, Vec<u8>)> {
         self.platform_text_system.rasterize_glyph(glyph_id)
     }
@@ -384,7 +384,7 @@ pub struct ShapedGlyph {
 }
 
 #[derive(Clone, Debug, PartialEq)]
-pub struct GlyphRasterizationParams {
+pub struct GlyphRasterParams {
     pub(crate) font_id: FontId,
     pub(crate) glyph_id: GlyphId,
     pub(crate) font_size: Pixels,
@@ -392,9 +392,9 @@ pub struct GlyphRasterizationParams {
     pub(crate) scale_factor: f32,
 }
 
-impl Eq for GlyphRasterizationParams {}
+impl Eq for GlyphRasterParams {}
 
-impl Hash for GlyphRasterizationParams {
+impl Hash for GlyphRasterParams {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.font_id.0.hash(state);
         self.glyph_id.0.hash(state);

crates/gpui3/src/window.rs 🔗

@@ -1,6 +1,6 @@
 use crate::{
     px, AnyView, AppContext, AvailableSpace, Bounds, Context, Effect, Element, EntityId, FontId,
-    GlyphId, GlyphRasterizationParams, Handle, Hsla, IsZero, LayoutId, MainThread, MainThreadOnly,
+    GlyphId, GlyphRasterParams, Handle, Hsla, IsZero, LayoutId, MainThread, MainThreadOnly,
     MonochromeSprite, Pixels, PlatformAtlas, PlatformWindow, Point, Reference, Scene, Size,
     StackContext, StackingOrder, Style, TaffyLayoutEngine, WeakHandle, WindowOptions,
     SUBPIXEL_VARIANTS,
@@ -16,7 +16,7 @@ pub struct AnyWindow {}
 pub struct Window {
     handle: AnyWindowHandle,
     platform_window: MainThreadOnly<Box<dyn PlatformWindow>>,
-    glyph_atlas: Arc<dyn PlatformAtlas<GlyphRasterizationParams>>,
+    glyph_atlas: Arc<dyn PlatformAtlas<GlyphRasterParams>>,
     rem_size: Pixels,
     content_size: Size<Pixels>,
     layout_engine: TaffyLayoutEngine,
@@ -181,7 +181,7 @@ impl<'a, 'w> WindowContext<'a, 'w> {
             x: (glyph_origin.x.0.fract() * SUBPIXEL_VARIANTS as f32).floor() as u8,
             y: (glyph_origin.y.0.fract() * SUBPIXEL_VARIANTS as f32).floor() as u8,
         };
-        let params = GlyphRasterizationParams {
+        let params = GlyphRasterParams {
             font_id,
             glyph_id,
             font_size,
@@ -190,17 +190,12 @@ impl<'a, 'w> WindowContext<'a, 'w> {
         };
 
         let raster_bounds = self.text_system().raster_bounds(&params)?;
-
         if !raster_bounds.is_zero() {
             let layer_id = self.current_layer_id();
-            let offset = raster_bounds.origin.map(Into::into);
-            let glyph_origin = glyph_origin.map(|px| px.floor()) + offset;
-            // dbg!(glyph_origin);
             let bounds = Bounds {
-                origin: glyph_origin,
+                origin: glyph_origin.map(|px| px.floor()) + raster_bounds.origin.map(Into::into),
                 size: raster_bounds.size.map(Into::into),
             };
-
             let tile = self
                 .window
                 .glyph_atlas