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

fix writing canvas past bounds #34

Merged
merged 12 commits into from May 5, 2019
Merged

fix writing canvas past bounds #34

merged 12 commits into from May 5, 2019

Conversation

labseven
Copy link
Collaborator

@labseven labseven commented May 3, 2019

It works nicely.
Also fixed not writing last line on the right and bottom.

fixes #31

@labseven labseven requested a review from newsch May 3, 2019 18:51
@labseven labseven changed the title fix writing canvas past bounds (fixes #31) fix writing canvas past bounds May 3, 2019
newsch
newsch previously approved these changes May 3, 2019
src/frontend.c Show resolved Hide resolved
src/cursor.c Outdated Show resolved Hide resolved
@newsch
Copy link
Member

newsch commented May 3, 2019

I get a segfault when the canvas is too short, but not when it is too thin.

@labseven
Copy link
Collaborator Author

labseven commented May 3, 2019

I get a segfault when the canvas is too short, but not when it is too thin.

Fixed by messing with some magic numbers in cursor and frontend


// draw fill in rest of window
for (int x = max_x; x < view_max_x; x++) {
for (int y = 0; y < view_max_y; y++) {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be:

Suggested change
for (int y = 0; y < view_max_y; y++) {
for (int y = max_y; y < view_max_y; y++) {

src/frontend.c Outdated

// draw canvas onto window
for (int x = 0; x <= max_x; x++) {
for (int y = 0; y <= max_y; y++) {
Copy link
Member

Choose a reason for hiding this comment

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

Starting at 0 and drawing at index +1 feels weird to me. Is this to get around the border width?

Copy link
Member

Choose a reason for hiding this comment

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

Or is this compensating for the window border?

@newsch newsch dismissed their stale review May 3, 2019 22:20

New questions

@newsch
Copy link
Member

newsch commented May 4, 2019

I took a fresh look at it today. Starting with a view at (0, 0) and a canvas of size 10x10 I only get a usable space of 9x9:

┌──────────────────
│123456789XXXXXXXXX
│2        XXXXXXXXX
│3        XXXXXXXXX
│4        XXXXXXXXX
│5        XXXXXXXXX
│6        XXXXXXXXX
│7        XXXXXXXXX
│8        XXXXXXXXX
│9        XXXXXXXXX
│XXXXXXXXXXXXXXXXXX
│XXXXXXXXXXXXXXXXXX
│XXXXXXXXXXXXXXXXXX
│XXXXXXXXXXXXXXXXXX

@newsch
Copy link
Member

newsch commented May 4, 2019

Looks like it's overwriting the last row/column:

┌────────────
│0        XXX
│         XXX
│         XXX
│         XXX
│         XXX
│         XXX
│         XXX
│         XXX
│        8XXX
│XXXXXXXXXXXX
│XXXXXXXXXXXX
│XXXXXXXXXXXX

To replicate:

diff --git a/src/frontend.c b/src/frontend.c
index 17fc043..e592c2a 100644
--- a/src/frontend.c
+++ b/src/frontend.c
@@ -51,9 +51,12 @@ int main(int argc, char *argv[]) {
   status_win = create_status_win();
 
   cursor = cursor_new();
-  Canvas *canvas = canvas_new_blank(1000, 1000);
+  Canvas *canvas = canvas_new_blank(10, 10);
+  canvas_scharyx(canvas, 0, 0, '0');
+  canvas_scharyx(canvas, 8, 8, '8');
+  canvas_scharyx(canvas, 9, 9, '9');
 
-  view = view_new_startpos(canvas, 300, 300);
+  view = view_new_startpos(canvas, 0, 0);
 
   // Enable keyboard mapping
   keypad(canvas_win, TRUE);
@@ -71,7 +74,7 @@ int main(int argc, char *argv[]) {
   State new_state = {
       .ch_in = 0,
       .cursor = cursor,
-      .current_mode = MODE_INSERT,
+      .current_mode = MODE_PAN,
       .last_arrow_direction = KEY_RIGHT,
       .last_canvas_mode = MODE_INSERT,
       .view = view,

@labseven
Copy link
Collaborator Author

labseven commented May 4, 2019

Yes I know my off-by-ones are messy. I'll clean it up today.

@newsch
Copy link
Member

newsch commented May 4, 2019

I got it working in 5253be5 but it doesn't draw to the edge of the screen anymore. I think I have a fix almost done.

@newsch
Copy link
Member

newsch commented May 4, 2019

Another bug I found was panning the screen past the end of the canvas overwrites the border.

@labseven
Copy link
Collaborator Author

labseven commented May 4, 2019

Ok, I cleaned it up and added bounds checking

@labseven
Copy link
Collaborator Author

labseven commented May 4, 2019

I fixed the "panning the screen past the end of the canvas overwrites the border." by checking for panning bounds. It will not let you pan past the edge anymore.

@newsch
Copy link
Member

newsch commented May 4, 2019

Looks good, but now the cursor can move to the bottom and right border again.
What are view_max_x and view_max_y supposed to represent?

@labseven
Copy link
Collaborator Author

labseven commented May 5, 2019

darn it. Let me dig deeper

Copy link
Member

@newsch newsch left a comment

Choose a reason for hiding this comment

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

Works for me. Do you want to rebase before merging it in?

@labseven labseven merged commit 70669f9 into master May 5, 2019
@newsch
Copy link
Member

newsch commented May 5, 2019

Looks like that had some duplicates of free-line. I don't think that will cause us any issues though.

@newsch newsch added this to In progress in Spring 2019 Project Tracking via automation May 5, 2019
@newsch newsch moved this from In progress to Done in Spring 2019 Project Tracking May 5, 2019
@labseven labseven deleted the adam/fix_canvasbounds branch May 8, 2019 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

If the canvas is smaller than the terminal window, characters are still written
3 participants