search

MEGA65 / mega65-rom-public

MEGA65 ROM public issue reporting
4 stars 0 forks source link

PEN hangs if a graphics screen isn't active, but was before #46

Closed dansanderson closed 1 year ago

dansanderson commented 1 year ago

Test Environment (required) You can use MEGA65INFO to retrieve this.

Describe the bug PEN hangs if a graphics screen isn't active, but was before.

To Reproduce

  1. PEN 3. Nothing happens.
  2. SCREEN 320,200,2:SCREEN CLOSE. Screen opens then immediately closes.
  3. PEN 3. Computer hangs.

Expected behavior PEN and all other graphics system commands that operate on an open screen should throw ?SCREEN NOT OPEN ERROR when a screen is not open. Most screen commands do this already. This includes both PEN cases described above.

Additional context Original forum64.de report.

It may not be worth trying to understand why PEN fails the way it does with a missing precondition. For what it's worth, the hang is in a loop near $A2DA in 920383, which is a junk location (a text message in the C64 kernel).

From Snoopy:

After adding "jsr CheckGraphicMode" right after "c65.setpen: " in line 18527 in b65.src will fix this issue.

We should vet the other graphics commands for similar behavior and make the implementation consistent. Most of the graphics system is implemented in c65.* and kg65.* routines in a sensible layout.

dansanderson commented 1 year ago

Notably address $A2DA is in the C64 BASIC ROM in the middle of an error message string, so that's probably unintentional. ;)

snoopy-f64 commented 1 year ago

PEN lacks the check if the graphics is on. If you use PEN directly after startup, you should get an error message that no graphics screen is open, because PEN is a graphic command. The three bytes that make the check should be included in PEN, then it works.

dansanderson commented 1 year ago

I confirmed that jsr CheckGraphicMode in c65.setpen is the correct fix. Nearly all graphics commands use either CheckGraphicMode or get_GKI.parm (which calls CheckGraphicMode) to abort with a ?SCREEN NOT OPEN ERROR if there is no active drawscreen, or the requested screen is not open. This is always called in the BASIC entry point and not the kg65 subroutine. I confirmed that all other BASIC 65 bitmap graphics commands and functions are appropriately guarded.

Notably, DMODE, DPAT, and RGRAPHIC() do not check for an open screen, because they operate normally without one. PEN and RPEN() need an open screen to handle screen palettes in bitplane modes, converting the provided color number to a global "color index" that is stored in the global kg65.drawpens.

I'm not sure I fully understand why it works this way. The documentation and API design implies that palettes are screen-specific and the pen number is global, which would imply that the pen number refers to the color number of the active drawscreen, and therefore wouldn't need an active drawscreen just to set the pen number (like DPAT et al). I would guess that's why PEN is currently unguarded. I can't get palettes to behave this way. Either there's a bug or design flaw in PALETTE screen,... or there's something I don't understand about the design intent.

Regardless, the current implementation of kg65.setpen calls kg65.s.color2index which needs an active drawscreen. So for now at least, this is the correct fix.