Opened 17 years ago
Closed 6 years ago
#7559 closed feature request (fixed)
BASS: Don't limit the screen height to 192 pixels
| Reported by: | jvprat | Owned by: | bonki |
|---|---|---|---|
| Priority: | normal | Component: | Engine: Sky |
| Version: | Keywords: | has-pull-request, original | |
| Cc: | Game: | Beneath a Steel Sky |
Description
It seems the whole game shows just 192 pixels of height, leaving a 8 pixels black bar at the bottom.
eriktorbjorn says it would be relatively easy to fix the intro, where there are some cut images (it's easy to notice in the images when joey is shot, the pink border is cut at the bottom).
It would also be nice to know if the game data, for example the backgrounds, can fill the 200 pixels and it can be enhanced or not.
Sometimes, when there are exits in the bottom of the screen, the mouse cursor can get to the border of the screen (over the black bar), but the text that follows the cursor is just cut at 192 pixels. It gives a strange feeling.
Thanks for your work!
Ticket imported from: #1689523. Ticket imported from: feature-requests/375.
Attachments (11)
Change History (36)
comment:1 by , 17 years ago
comment:2 by , 17 years ago
| Owner: | set to |
|---|
comment:3 by , 17 years ago
Not as simple as I had hoped, but I'm attaching an experimental patch that shows the "fullscreen" images in the cases where it doesn't cause animation glitches. File Added: sky-hack.diff
comment:4 by , 17 years ago
After further examining (and also thanks to the patch) I've found that the problem comes from Screen::processSequence().
If I set GAME_SCREEN_HEIGHT to 200 and limit processSequence to work with just 192 rows, it works and shows the images at full screen. The attached patch shows this.
This leaves some dirty areas (which were manually cleared in eriktorbjorn's patch), but they're there only when the screen is updated by processSequence. The images that appear with some kind of animation, do it through this function, and it's currently limited (by my patch) to 192 rows, which means these images are still cut, and they can't cover the bottom of the screen.
I think the way to go is trying to do processSequence work with 200 rows. I've had no time to watch into this. Maybe this weekend.
Setting GAME_SCREEN_HEIGHT to 200 makes the game unplayable (it crashes when changing some scenes), so it will need more work to get a fully functional patch. File Added: sky-hack2.diff
comment:5 by , 17 years ago
Yes, that's a simpler approach, of course. The problem I had when making all the intro backgrounds use the entire screen was that some of them interfered with the animation to some extent: The cars at the opening of the floppy intro, the shadow at the end of the helicopter crash, garbage left after some scene transitions, etc. (I marked such images with a comment in my patch, as you probably saw.) And as I recall it, there was one where the bottom part was just solid red.
That's why I opted for displaying the full image on a case-by-case basis.
comment:6 by , 17 years ago
Those sequences _are_ 320x192 pixels in size and there is no way to make them work with 200 lines, unless you draw the remaining images manually and patch them into the datafiles. The games was simply made to run at that resolution and it does that in all areas of the game. Trying to make a few images show 8 more lines and thereby even causing graphical glitches feels rather pointless to me.
comment:7 by , 17 years ago
After carefully reading the code I've also realized that the sequences depend on the game data. I thought they might contain transition commands like "scroll down" on static images (in which case it could have been enhanced), instead of some kind of video. It's a pity we can't enjoy the extra 8 pixels of this game, even if some parts are there ;) In this case, the first patch is all we can hope.
I'll try to see if we can enhance at least some backgrounds in-game, or if it's really pointless.
Thanks for your comments.
comment:8 by , 17 years ago
> Trying to make a few images show 8 more lines and > thereby even causing graphical glitches feels rather > pointless to me.
That's why I limited my patch to the intro images that, as far as I could tell after repeated viewings of both intros, could be shown without causing any glitches. I deemed even minor glitches to be unacceptable.
Though the CLEARGFX directive turned out to be a bit of an overkill since I use it only to clear the same area of the screen, and only right between FADEDOWN and SHOWSCREEN. I'll submit an updated patch later tonight, probably.
by , 17 years ago
| Attachment: | sky-hack3.diff added |
|---|
Slightly simplified version of original approach
comment:9 by , 17 years ago
It was possible to simplify it slightly, as I thought/hoped. New patch attached. File Added: sky-hack3.diff
comment:10 by , 17 years ago
I'm attaching a slightly modified version of the 3rd patch with two changes: - In some cases, CLEARSCREEN + SHOWSCREEN could be changed to SHOWSCREEN | FULLSCREEN (no need for an extra screen update). - The cockpit scene can also be shown in fullscreen. The spanish subtitles in the floppy intro get lower than row 192 and it shows a strange effect if the image isn't shown in fullscreen. File Added: sky-hack3b.diff
comment:11 by , 17 years ago
Here comes another change.
I've managed to show 2 more fullscreen images (when the old man talks to Reich and Foster watching the security symbol) by mimicking the sequences by cleaning the bottom of the screen when needed, making the graphic glitches almost unnoticeable. Feel free to ignore this patch if you feel the result isn't acceptable. File Added: sky-hack4.diff
comment:12 by , 17 years ago
> - In some cases, CLEARSCREEN + SHOWSCREEN could be changed > to SHOWSCREEN | FULLSCREEN (no need for an extra screen update).
I thought of doing it that way, but it seemed counter-intuitive to flag background images as 'fullscreen' unless they actually used the bottom part of the screen.
> The spanish subtitles in the floppy intro get lower than row > 192 and it shows a strange effect if the image isn't shown in > fullscreen.
Interesting... Incidentally, the original draws spanish subtitles below the 192nd pixel too, but it's less noticeable there.
comment:13 by , 17 years ago
Lavosspawn fixed the subtitle problem, accidentally breaking the intro patches in the process. I've updated my own latest patch, though. There's one slight change in that I now return true in the CLEARSCREEN case, instead of using break. That's just to be consistent with the surrounding code, and shouldn't make any difference.
File Added: sky-hack3-updated.diff
comment:16 by , 17 years ago
I don't think the helicopter cockpit has to be fullscreen any more, with lavosspawn's subtitle fix.
comment:17 by , 17 years ago
This is just an updated version of sky-hack4-updated.diff, with the "fullscreen" flag removed from the helicopter interior. As far as I can tell, it's not needed.
I've watched both the floppy and CD intros, and as far as I can tell the patch adds no new glitches to either of them. The way it's written, it shouldn't affect the rest of the game at all.
However, I don't want to make any decisions about it since I'm not the engine maintainer. File Added: sky-hack4b.diff
comment:19 by , 17 years ago
The last thing I heard was that lavosspawn didn't seem very enthusiastic about them, though I don't remember why.
comment:20 by , 6 years ago
| Component: | → Engine: Sky |
|---|---|
| Game: | → Beneath a Steel Sky |
by , 6 years ago
| Attachment: | scummvm00009.png added |
|---|
This issue is about there being a small black rectangle at the bottom of the screen. Attached is a picture showing this.
comment:21 by , 6 years ago
Eleven years later, how do you feel about this? Should we drop or merge this @eriktorbjorn?
comment:22 by , 6 years ago
I still don't remember what the objections were, but I don't really mind either way. The patch only affects the game intro (mainly the CD intro), where some images are somewhat noticeably cut off. It shouldn't affect the rest of the game at all.
The patch doesn't apply cleanly any more, so I'm attaching an updated version. I've given it a quick test, and it seems to work.
comment:23 by , 6 years ago
Thanks for your quick reply. In its latest form this looks rather harmless to me. I cannot imagine that not displaying the images in full resolution was intentional so I vote for merging this.
I opened PR-1232 in your name and will merge this unless someone objects during the next couple of days.
comment:24 by , 6 years ago
| Keywords: | has-pull-request original added |
|---|
comment:25 by , 6 years ago
| Owner: | changed from to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
Merged in commit 0ffb057b66db7cb8319bec1c5e54067e6865c9d6.
To clarify slightly, this glitch appears to be present in the original as well.
It's very easy to get the intro to draw the bottom rows of the images that are displayed by Screen::showScreen(uint16 fileNum) (the SHOWSCREEN directive in the sequence data). I don't know if all the images displayed by that function are 200 pixels tall, but that can easily be checked by looking at the size of the data that has just been loaded.
The trick seems to be to get it to erase the bottom rows when they are no longer desired. The fix I had in mind when I said it was relatively easy leaves garbage during some of the "dissolve" effects. (Are these the ones started by the BGFLIRT directive?)