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 background select interpretation and 0:1 aspect ratio #6

Merged
merged 4 commits into from
Aug 9, 2021

Conversation

j4james
Copy link
Contributor

@j4james j4james commented Aug 8, 2021

This PR fixes a bunch of issues in the way the background select is interpreted following our discussions in issue #5. It also includes a fix for the 0:1 aspect ratio which I had previously assumed to be an error (which would fallback to the macro parameter value), but is actually just rounded up to a minimum AR of 1:1.

j4james added 4 commits August 8, 2021 20:11
While a 0:0 aspect ratio is treated as an error, and falls back to the
macro parameter value, a 0:1 aspect ratio is considered valid, and will
round up to the minimum AR of 1:1.
The background fill only begins on the first line that outputs a sixel
data value. So if the image starts with one or more DECGNL commands,
they will need to be prefixed with at least one data value (potentially
zero) in order to trigger a fill from the very top of the image.
The vertical extent specified in the raster attributes command should
be given in screen pixels rather than sixel pixels.
This replaces the original (incorrect) zero size test with a test of
the background fill when an image begins with one or more DECGNL
commands. And the default/zero size test is incorporated into the last
test case, which is intended to fill up to the screen boundaries.
Copy link
Owner

@hackerb9 hackerb9 left a comment

Choose a reason for hiding this comment

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

Ah! I had run into that problem with DECGNL not taking effect. So you're using ? to draw an empty sixel ("graphics space"), which seems reasonable. This is reminiscent of how we had to have a character to get the proper region to clear instead of the whole screen. I had been appreciating " for fixing that problem because of the nice symmetry with DECGRA, but it doesn't help in this case, whereas ? works for both.

Is there any other character or sequence that might be more generally useful as the "solution" for getting fill to do the right thing? For example, adding graphics carriage return, ?$, wouldn't ever be necessary, right?

@hackerb9 hackerb9 merged commit 473b8cb into hackerb9:main Aug 9, 2021
@hackerb9
Copy link
Owner

hackerb9 commented Aug 9, 2021

Updated .png files are available with the results.

@j4james
Copy link
Contributor Author

j4james commented Aug 9, 2021

Is there any other character or sequence that might be more generally useful as the "solution" for getting fill to do the right thing? For example, adding graphics carriage return, ?$, wouldn't ever be necessary, right?

Oh you mean always putting that at the start of the image when you don't know what might follow? That's not a bad idea. Although if you're optimising images to the extent that you're getting a solitary DECGNL at the start of the image, you'd probably want to add a special case check for that and only output the ? when absolutely necessary.

Updated .png files are available with the results.

Thanks for that, but what happened to the color_selection output? The text background is black now, and it doesn't look like anything has changed in the test script. Could that be an issue with the screen capture script?

@hackerb9
Copy link
Owner

You're right. It should have been blue periods on white. I just ran the script again and it is white, not black, so I'm not sure what happened.

I'll try running Media Copy again in 132- and 80-column mode and see if I can track it down. (As you may recall, background_select.sh in 80 column mode was the first screen image I found that consistently caused the VT340 to glitch when running Media Copy).

@j4james
Copy link
Contributor Author

j4james commented Aug 10, 2021

You're right. It should have been blue periods on white. I just ran the script again and it is white, not black, so I'm not sure what happened.

OK cool. My one thought was the DECGPBM mode wasn't set (Graphics Print Background), but it looks like the mediacopy script already enables that automatically, so I don't know. Maybe it's just another glitch in the hardware.

@j4james j4james deleted the fix-background-fill branch August 10, 2021 10:27
hackerb9 added a commit that referenced this pull request Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants