Skip to content

Commit 5788208

Browse files
committed
Fixed bug in ansi output.
1 parent e97186e commit 5788208

File tree

5 files changed

+113
-48
lines changed

5 files changed

+113
-48
lines changed

crates/icy_draw/src/ui/editor/ansi/main_area.rs

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -565,8 +565,39 @@ impl AnsiEditorMainArea {
565565
}
566566

567567
/// Scroll the canvas to show the given character position (used for goto user)
568-
pub fn scroll_to_position(&mut self, col: i32, row: i32) {
569-
self.core.scroll_to_position(col, row);
568+
pub fn scroll_to_position(&mut self, col: i32, row: i32) -> Task<AnsiEditorMessage> {
569+
let (buf_width, buf_height) = {
570+
let mut screen_guard = self.core.screen.lock();
571+
let state = screen_guard
572+
.as_any_mut()
573+
.downcast_mut::<EditState>()
574+
.expect("AnsiEditor screen should always be EditState");
575+
let buffer = state.get_buffer();
576+
(buffer.width().max(1) as f32, buffer.height().max(1) as f32)
577+
};
578+
579+
let norm_x: f32 = (col as f32 + 0.5) / buf_width;
580+
let norm_y: f32 = (row as f32 + 0.5) / buf_height;
581+
582+
let terminal = &self.core.canvas.terminal;
583+
let content_width = terminal.content_width().max(1.0);
584+
let content_height = terminal.content_height().max(1.0);
585+
let visible_width = terminal.visible_content_width().max(1.0);
586+
let visible_height = terminal.visible_content_height().max(1.0);
587+
588+
// Keep current X when horizontal scrolling isn't possible.
589+
let current_scroll_x = terminal.scroll_x();
590+
591+
let target_x = if content_width > visible_width {
592+
norm_x.clamp(0.0, 1.0) * content_width - visible_width / 2.0
593+
} else {
594+
current_scroll_x
595+
};
596+
597+
let target_y = norm_y.clamp(0.0, 1.0) * content_height - visible_height / 2.0;
598+
599+
// Programmatic scrolling is task-driven via the owning scroll_area.
600+
terminal.scroll_to_content::<AnsiEditorMessage>(Some(target_x), Some(target_y))
570601
}
571602

572603
pub fn zoom_info_string(&self) -> String {
@@ -1548,7 +1579,25 @@ impl AnsiEditorMainArea {
15481579
RightPanelMessage::Minimap(minimap_msg) => {
15491580
match minimap_msg {
15501581
MinimapMessage::ScrollTo { norm_x, norm_y } => {
1551-
self.core.scroll_canvas_to_normalized(norm_x, norm_y);
1582+
let terminal = &self.core.canvas.terminal;
1583+
let content_width = terminal.content_width().max(1.0);
1584+
let content_height = terminal.content_height().max(1.0);
1585+
let visible_width = terminal.visible_content_width().max(1.0);
1586+
let visible_height = terminal.visible_content_height().max(1.0);
1587+
1588+
// Keep current X when horizontal scrolling isn't possible.
1589+
let current_scroll_x = terminal.scroll_x();
1590+
1591+
let target_x = if content_width > visible_width {
1592+
norm_x.clamp(0.0, 1.0) * content_width - visible_width / 2.0
1593+
} else {
1594+
current_scroll_x
1595+
};
1596+
1597+
let target_y = norm_y.clamp(0.0, 1.0) * content_height - visible_height / 2.0;
1598+
1599+
let scroll_task = terminal.scroll_to_content::<AnsiEditorMessage>(Some(target_x), Some(target_y));
1600+
return Task::batch(vec![task, scroll_task]);
15521601
}
15531602
MinimapMessage::Scroll(_dy) => {
15541603
// handled internally by the minimap view

crates/icy_draw/src/ui/editor/ansi/widget/minimap/minimap_shader.rs

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -409,19 +409,10 @@ impl shader::Program<MinimapMessage> for MinimapProgram {
409409
cursor: mouse::Cursor,
410410
) -> Option<icy_ui::widget::Action<MinimapMessage>> {
411411
match event {
412-
// Handle redraw requests for continuous drag scrolling
413-
icy_ui::Event::Window(icy_ui::window::Event::RedrawRequested(now)) => {
414-
if state.is_dragging {
415-
state.last_redraw = Some(*now);
416-
// Re-send scroll position based on last known pointer position
417-
if let Some(last_pos) = state.last_pointer_position {
418-
if let Some((norm_x, norm_y)) = self.calculate_normalized_position(last_pos, bounds) {
419-
return Some(icy_ui::widget::Action::publish(MinimapMessage::ScrollTo { norm_x, norm_y }));
420-
}
421-
}
422-
} else {
423-
state.last_redraw = None;
424-
}
412+
// IMPORTANT: Do not publish messages from RedrawRequested.
413+
// Doing so can create redraw → message → layout invalidation loops in winit.
414+
icy_ui::Event::Window(icy_ui::window::Event::RedrawRequested(_now)) => {
415+
state.last_redraw = None;
425416
}
426417

427418
// Handle mouse button press - start dragging

crates/icy_draw/src/ui/editor/ansi/widget/minimap/mod.rs

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -124,20 +124,22 @@ impl MinimapView {
124124
)
125125
}
126126

127-
/// Ensure the viewport rectangle is visible in the minimap
128-
/// Adjusts scroll_position so the viewport is always on screen
129-
/// Uses interior mutability so it can be called from view()
127+
/// Compute the scroll position (0..1) that would keep the viewport rectangle visible.
130128
///
131-
/// viewport_y and viewport_height are normalized (0-1) relative to full buffer
132-
/// content_width and content_height are in pixels
133-
fn ensure_viewport_visible(&self, viewport_y: f32, viewport_height: f32, content_width: f32, content_height: f32) {
134-
let shared = self.shared_state.lock();
135-
let avail_width = shared.available_width;
136-
let avail_height = shared.available_height;
137-
drop(shared);
138-
129+
/// This is intentionally **pure** (does not mutate `self.scroll_position`) so it can be
130+
/// called from `view()` without triggering layout invalidation/redraw loops.
131+
fn compute_scroll_to_show_viewport(
132+
&self,
133+
current_scroll: f32,
134+
viewport_y: f32,
135+
viewport_height: f32,
136+
content_width: f32,
137+
content_height: f32,
138+
avail_width: f32,
139+
avail_height: f32,
140+
) -> f32 {
139141
if avail_width <= 0.0 || avail_height <= 0.0 || content_height <= 0.0 || content_width <= 0.0 {
140-
return;
142+
return current_scroll;
141143
}
142144

143145
// Scale factor: minimap fills available width
@@ -148,21 +150,20 @@ impl MinimapView {
148150

149151
// If content fits in available space, no scrolling needed
150152
if scaled_content_height <= avail_height {
151-
*self.scroll_position.borrow_mut() = 0.0;
152-
return;
153+
return 0.0;
153154
}
154155

155-
// viewport_y and viewport_height are already normalized (0-1)
156+
// viewport_y and viewport_height are normalized (0-1)
156157
// Convert to scaled pixels
157158
let viewport_top_scaled = viewport_y * content_height * scale;
158159
let viewport_height_scaled = viewport_height * content_height * scale;
159160
let viewport_bottom_scaled = viewport_top_scaled + viewport_height_scaled;
160161

161162
// Maximum scroll offset in pixels
162-
let max_scroll_px = scaled_content_height - avail_height;
163+
let max_scroll_px = (scaled_content_height - avail_height).max(0.0);
163164

164165
// Current scroll offset in pixels
165-
let current_scroll = *self.scroll_position.borrow();
166+
let current_scroll = current_scroll.clamp(0.0, 1.0);
166167
let current_scroll_px = current_scroll * max_scroll_px;
167168

168169
// Visible range in scaled pixels
@@ -172,24 +173,20 @@ impl MinimapView {
172173
// If viewport is larger than visible area, align to viewport top to avoid oscillation
173174
if viewport_height_scaled >= avail_height {
174175
let new_scroll_px = viewport_top_scaled;
175-
let new_scroll = (new_scroll_px / max_scroll_px).clamp(0.0, 1.0);
176-
*self.scroll_position.borrow_mut() = new_scroll;
177-
return;
176+
return (new_scroll_px / max_scroll_px).clamp(0.0, 1.0);
178177
}
179178

180179
// Check if viewport is above visible area
181180
if viewport_top_scaled < visible_top {
182-
// Scroll up to show viewport at top
183181
let new_scroll_px = viewport_top_scaled;
184-
let new_scroll = (new_scroll_px / max_scroll_px).clamp(0.0, 1.0);
185-
*self.scroll_position.borrow_mut() = new_scroll;
182+
(new_scroll_px / max_scroll_px).clamp(0.0, 1.0)
186183
} else if viewport_bottom_scaled > visible_bottom {
187-
// Scroll down to show viewport at bottom
188184
let new_scroll_px = viewport_bottom_scaled - avail_height;
189-
let new_scroll = (new_scroll_px / max_scroll_px).clamp(0.0, 1.0);
190-
*self.scroll_position.borrow_mut() = new_scroll;
185+
(new_scroll_px / max_scroll_px).clamp(0.0, 1.0)
186+
} else {
187+
// If viewport is within visible area, don't change scroll
188+
current_scroll
191189
}
192-
// If viewport is within visible area, don't change scroll
193190
}
194191

195192
/// Update the minimap view state
@@ -255,12 +252,24 @@ impl MinimapView {
255252
let content_width = content_width_u32 as f32;
256253
let content_height = content_height_f32;
257254

258-
// Auto-scroll to keep viewport visible
259-
self.ensure_viewport_visible(viewport_info.y, viewport_info.height, content_width, content_height);
255+
// Compute a scroll position that keeps the viewport visible.
256+
// Do NOT mutate widget state from view() (can trigger layout invalidation loops).
257+
let scroll_normalized = {
258+
let current = *self.scroll_position.borrow();
259+
self.compute_scroll_to_show_viewport(
260+
current,
261+
viewport_info.y,
262+
viewport_info.height,
263+
content_width,
264+
content_height,
265+
avail_width,
266+
avail_height,
267+
)
268+
};
260269

261270
// Calculate which tiles to select based on visible area
262271
let tile_height = TILE_HEIGHT as f32;
263-
let scroll_normalized = *self.scroll_position.borrow();
272+
let scroll_normalized = scroll_normalized;
264273

265274
let max_tile_idx = ((content_height / tile_height).ceil().max(1.0) as i32) - 1;
266275

crates/icy_draw/src/ui/main_window/main_window.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1966,7 +1966,9 @@ impl MainWindow {
19661966
if let Some((col, row)) = user.cursor {
19671967
log::info!("Goto user {} at ({}, {})", user.user.nick, col, row);
19681968
if let ModeState::Ansi(editor) = &mut self.mode_state {
1969-
editor.scroll_to_position(col, row);
1969+
// Clicking elsewhere removes focus from chat input
1970+
self.collaboration_state.chat_input_focused = false;
1971+
return editor.scroll_to_position(col, row).map(Message::AnsiEditor);
19701972
}
19711973
}
19721974
}

crates/icy_engine/src/formats/ansi_v2.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,7 @@ impl StringGeneratorV2 {
777777
fn generate_cells<T: TextPane>(&self, buf: &TextBuffer, layer: &T, area: Rectangle, font_map: &HashMap<u8, u8>) -> (AnsiState, Vec<Vec<CharCell>>) {
778778
let mut result: Vec<Vec<CharCell>> = Vec::new();
779779

780-
let mut state = AnsiState {
780+
let default_state = AnsiState {
781781
is_bold: false,
782782
is_blink: false,
783783
is_italic: false,
@@ -792,9 +792,18 @@ impl StringGeneratorV2 {
792792
bg_idx: 0,
793793
};
794794

795+
let mut state = default_state.clone();
796+
795797
for y in area.y_range() {
796798
let mut line = Vec::new();
797799

800+
// In UTF-8 mode we emit SGR resets between lines (see `generate()`), so we must
801+
// also reset the exporter state between lines. Otherwise the next line may start
802+
// without re-emitting attributes (e.g. bold/colors) after an `ESC[0m`.
803+
if self.level.supports_utf8() {
804+
state = default_state.clone();
805+
}
806+
798807
if self.options.longer_terminal_output {
799808
if let Some(skip_lines) = &self.options.skip_lines {
800809
if skip_lines.contains(&(y as usize)) {
@@ -961,9 +970,14 @@ impl StringGeneratorV2 {
961970
continue;
962971
}
963972
}
973+
// In longer-terminal-output mode we position each line explicitly.
974+
// For UTF-8 terminal output we also reset SGR for each emitted line to avoid
975+
// attribute bleed and to match the per-line state reset in `generate_cells()`.
964976
if is_first_output_line {
965977
is_first_output_line = false;
966978
result.extend_from_slice(b"\x1b[0m");
979+
} else if self.level.supports_utf8() {
980+
result.extend_from_slice(b"\x1b[0m");
967981
}
968982
result.extend_from_slice(b"\x1b[");
969983
result.extend_from_slice((y + 1).to_string().as_bytes());

0 commit comments

Comments
 (0)