(WIP) feature: VESA VBE 1.2#1587
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements VESA BIOS Extension (VBE) 1.2 support for the VGA BIOS emulator, enabling compatibility with games like Rules of Engagement 2 that require Super VGA functionality. The implementation adds standardized VESA functions for querying and setting video modes beyond standard VGA capabilities.
Key Changes:
- Implements VBE 1.2 specification with three core functions: GetControllerInfo (00h), GetModeInfo (01h), and SetMode (02h)
- Adds memory-mapped data structures (VbeInfoBlock, VbeModeInfoBlock) for VESA information exchange
- Refactors existing VGA BIOS code to replace magic numbers with named constants for improved maintainability
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
VgaBios.cs |
Adds VBE 1.2 implementation with three new functions, two nested data structure classes, and extensive constant definitions. Refactors existing code to use named constants instead of magic numbers. |
IVesaBiosExtension.cs |
Defines the interface contract for VESA BIOS Extension functions with comprehensive documentation from the VBE 1.2 specification. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (bpp == 15 || bpp == 16) { | ||
| modeInfo.RedMaskSize = VbeModeInfoConstants.RedGreenBlueMaskSize; | ||
| modeInfo.RedFieldPosition = (byte)(bpp == 16 ? 11 : 10); | ||
| modeInfo.GreenMaskSize = (byte)(bpp == 16 ? VbeModeInfoConstants.GreenMaskSize6Bit : VbeModeInfoConstants.RedGreenBlueMaskSize); |
Check warning
Code scanning / CodeQL
Cast to same type
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix this kind of issue, remove the redundant cast so that the code directly assigns the expression of the already-correct type. This keeps the code cleaner and avoids suggesting a nonexistent type conversion.
Concretely, in src/Spice86.Core/Emulator/InterruptHandlers/VGA/VgaBios.cs, within VbeGetModeInfo, update the line assigning modeInfo.GreenMaskSize. The ternary already produces a byte, so the explicit (byte) cast should be removed. The corrected line will simply assign the result of the conditional expression. No additional imports, methods, or definitions are required, and no other logic in the method needs to change.
| @@ -1532,7 +1532,7 @@ | ||
| if (bpp == 15 || bpp == 16) { | ||
| modeInfo.RedMaskSize = VbeModeInfoConstants.RedGreenBlueMaskSize; | ||
| modeInfo.RedFieldPosition = (byte)(bpp == 16 ? 11 : 10); | ||
| modeInfo.GreenMaskSize = (byte)(bpp == 16 ? VbeModeInfoConstants.GreenMaskSize6Bit : VbeModeInfoConstants.RedGreenBlueMaskSize); | ||
| modeInfo.GreenMaskSize = bpp == 16 ? VbeModeInfoConstants.GreenMaskSize6Bit : VbeModeInfoConstants.RedGreenBlueMaskSize; | ||
| modeInfo.GreenFieldPosition = 5; | ||
| modeInfo.BlueMaskSize = VbeModeInfoConstants.RedGreenBlueMaskSize; | ||
| modeInfo.BlueFieldPosition = 0; |
maximilien-noal
force-pushed
the
feat/VBE_VESA_1.2
branch
from
5aa17bf to
290c7b5
Compare
maximilien-noal
force-pushed
the
feat/VBE_VESA_1.2
branch
2 times, most recently
from
30a8243 to
88734e1
Compare
maximilien-noal
force-pushed
the
feat/VBE_VESA_1.2
branch
2 times, most recently
from
1b5c29d to
3ac5052
Compare
|
@copilot #1587 (review) implement changes based on this feedback |
|
@maximilien-noal I've opened a new pull request, #1978, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
You can also share your feedback on Copilot code review. Take the survey.
|
@copilot implement changes based on this feedback #1587 (review) |
|
@maximilien-noal I've opened a new pull request, #1983, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
You can also share your feedback on Copilot code review. Take the survey.
maximilien-noal
force-pushed
the
feat/VBE_VESA_1.2
branch
from
758bd0c to
c33b424
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description of Changes
Introduces VBE VESA 1.2 extensions to the VGA BIOS.
Rationale behind Changes
Some games try to use it (like Rules of Engagement 2)
Suggested Testing Steps
Foud no game that actually requires it, and its only an extension. Maybe a VBE utility ?