-
Notifications
You must be signed in to change notification settings - Fork 192
Crash Screen Updates #888
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
base: develop/2.4.0
Are you sure you want to change the base?
Crash Screen Updates #888
Conversation
|
can we also remove those stupid shadow instructions that appear behind the real ones. whats up with that |
Probably just a meme with crash screen transparency and scrolling. Also that asymmetrical box drawing thing has been a thing for a century (though that should probably get corrected already, why is that still this broken...) |
|
Ready for Review!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will review the shiny new denser things later (and probably gloss over the copied libdragon stuff if you can point me to which is which)
|
Also since we're touching it anyway, can we replace the HackerSM64 |
|
switching assert strategies seems like a rabbit hole since it's either a trap, break, or syscall depending on who you ask (it's even a |
|
The issue isn't using |
|
That seems like an easy enough fix, though some time down the line (read: |
|
Good idea. My suggestion would be |
|
Ready for another lap 🏇 |
gheskett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code-wise, I think I'm good on this for now. That said, I've found some issues through playtesting:
- The disasm seems to have an issue in which the functions disappear upon scrolling (see screenshot below for difference after scrolling down a singular entry). Notice the first instruction was skipped, and the other function name is gone. The instructions print one fewer lines than before.
- The Puppyprint log page does not display correctly. It need to not re-render the background, and also prints bottom-up rather than top-down.
- Some of the text alignments on the assert page (and also page 0) are still off. Ideally everything is monospace.
- The crash screen seems to not be able to find audio code files (in this case,
src/audio/synthesis.c). Notice also that the stack trace is incomplete and only displays two functions.
|
Fixed spacing and the puppyprint visual bug. (Probably not going to fix the puppyprint log ordering, since that seems like an intentional feature, see here) For the stack bug, I was able to get just before |
|
Ready for another lap 🏇 |
gheskett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple things:
- I'm noticing the spacing is still off for some things, notably following
File: - The synthesis stack trace still only goes back a couple functions (assuming this is still WIP)
- The Puppyprint log should be displayed at the top. Puppyprint does indeed do this:
I suppose the text printing order is a thing, but I honestly don't like how it prints newest at the top anyway, so that should be addressed Puppyprint-side. The only consideration there would be excess messages printing above the top. Fixing this would be easy if Puppyprint can't do newlines, otherwise we'd have to newline count everything ahead of time to make a determination. Probably out of scope, I won't require the ordering swap for this PR (though you're welcome to implement it if you want).
gheskett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I think I'm satisfied. The only thing I'm thinking now is that it might be a good idea to render 100% opacity on page swaps and render transparent only on the first render of the crash screen, since subsequent renders basically black out the screen already and just make text hard to read. Ideally we'd just cache the framebuffer here or something, but that's out of scope.
gheskett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DId some additional testing; the crash screen seems like it straight up does not work on console.
|
|
||
| // get the start of the current frame's func | ||
| while (!ADDRENTRY_IS_FUNC(funcstart) && !ADDRENTRY_IS_INLINE(funcstart)) { | ||
| funcstart = symt_addrtab_entry(&symt, --idx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried bisecting the (probably) crash, turns out this line seems to be the point of failure (or really, the IO_READ macro behind this function call). Absolutely no idea what's going on here though or how to fix it, so good luck :D
NOTE: No, it's not a null pointer exception with symt, but it may be possible that the IO_READ provided address itself is invalid. Worth noting also that the crash happens on the very first call to this function within the loop...assuming it's even a crash in the first place (yielded thread?)
Implement remaining branch instructions ✅
Implement COP1 instructions and
syscall✅Implement
.symformat andn64symtool from libdragon ✅File/line support as a result ✅
Change font to something with lowercase and symbols ✅
Move elements around to fill the whole screen ✅
Arbitrary text coloring ✅
Simple Overview page with only what's necessary ✅
Implement
break cache eret sync(Not sure if really necessary)