Skip to content

Fix POSTSCRIPTSTREAM to refine or default values for COLOR. #2177

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

Merged

Conversation

MattHeffron
Copy link
Contributor

(Don't require them to be FLOATP.)

This resolves #2176.

@MattHeffron MattHeffron requested a review from rmkaplan June 1, 2025 00:38
@MattHeffron MattHeffron self-assigned this Jun 1, 2025
@MattHeffron MattHeffron added the bug Something isn't working (as per documentation) label Jun 1, 2025
@rmkaplan
Copy link
Contributor

rmkaplan commented Jun 1, 2025 via email

@nbriggs
Copy link
Contributor

nbriggs commented Jun 1, 2025

@rmkaplan there are a few things in can be, but one option is that it's something that is COLORNUMBERP, which is defined on library/LLCOLOR.

@MattHeffron
Copy link
Contributor Author

Are integer colors like this some sort of index into a table of colors?

COLORNUMBERP checks against the color map for the screen, so I don't know what it would do for a B/W screen, nor for an imagestream that isn't to the screen.

This is the BRUSHCOLOR of a curve in the second embedded sketch. It appears that color 7 is specified in the sketch, but I don't see how to set the color of a BRUSH to use in the sketch. Is it actually a TEXTURE? So, I don't know how to find out where that "color" came from, and what it should do.
It looks like I might have to dig into SKETCH. How come TEDIT has no problem with that?

@rmkaplan
Copy link
Contributor

rmkaplan commented Jun 1, 2025 via email

@MattHeffron MattHeffron marked this pull request as draft June 2, 2025 19:22
@MattHeffron
Copy link
Contributor Author

I did a bunch of digging through the source code and discovered:

  • When a drawing operation targets the display, the BRUSHCOLOR is ignored if the target is 1BPP ScreenBitMap.
    If the target is ColorScreenBitMap , then BRUSHCOLOR is set into a bitmap of the same BPP as ColorScreenBitMap with no further interpretation, leaving that to the use of the ColorScreenBitMap.
  • In INTERPRESS the function \SETBRUSH.IP ignores the BRUSHCOLOR if it is a NUMBERP that's <= 0.
    (It doesn't do this safely. There are a couple of points there where the BRUSH function argument is used as a BRUSH record type, but it hasn't verified that it is that type.)

So, I propose that POSTSCRIPTSTREAM should behave similar to INTERPRESS and just ignore a BRUSHCOLOR if it is a SMALLP that's neither 0 nor 1, and use the current DSPCOLOR of the stream. If someone wants a specific color, or grayscale, they can use the color descriptions as documented in POSTSCRIPTSTREAM.TEDIT.
This seems the best alternative, both for consistency with INTERPRESS and simplicity.

@rmkaplan @hjellinek @masinter Any comments/objections?

@MattHeffron MattHeffron changed the title Fix POSTSCRIPTSTREAM to accept SMALLP values 0 and 1 as COLOR. Fix POSTSCRIPTSTREAM to refine or default values for COLOR. Jun 5, 2025
…hout other context (e.g., as an index into a color map).

POSTSCRIPTSTREAM should behave similar to INTERPRESS and just ignore a BRUSHCOLOR if it is a SMALLP that's neither 0 nor 1, and use the current DSPCOLOR of the stream.
If someone wants a specific color, or grayscale, they can use the color descriptions as documented in POSTSCRIPTSTREAM.TEDIT.
@MattHeffron MattHeffron requested review from masinter and hjellinek June 5, 2025 23:33
@MattHeffron MattHeffron marked this pull request as ready for review June 5, 2025 23:35
Copy link
Contributor

@rmkaplan rmkaplan left a comment

Choose a reason for hiding this comment

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

I tried it, and it worked fine on the problematic equation file. But the paragraph in the documentation beginning "Anywhere that a texture or color..." seems to only allow 0 and 1, should that be updated to describe the other pass-through possibilities ?

@MattHeffron MattHeffron requested a review from rmkaplan June 16, 2025 19:37
Copy link
Contributor

@rmkaplan rmkaplan left a comment

Choose a reason for hiding this comment

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

LGTM

@rmkaplan rmkaplan merged commit 72032af into master Jun 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working (as per documentation)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

POSTSCRIPT doesn't take 0 as a color
3 participants