Macintosh classic/carbon port - #392
RandoOnSteam wants to merge 7 commits into
Conversation
GitMensch
left a comment
There was a problem hiding this comment.
This is definitely an interesting port - thanks for the PR!
I guess you've executed all tests locally and found them working "good enough"? Anything to watch out for?
The main missing part is the port's README.md, please add it.
Can you please remove the CodeWarrior only files? They are likely quite useful to you, but so are definitions for Code::Blocks for others - which we don't keep in the repo either.
I've only reviewed the more "common" files and think the TickCount should only be used in the helper function (which, according to the spec I've found, seems to need a number switch).
As noted: looks promising (the main issue is a likely not easy way to test that, but if it works for you...).
| #include <OSUtils.h> | ||
| long PDC_millisecs( void) | ||
| { | ||
| return (((long)TickCount()) * 1000) / 60; |
There was a problem hiding this comment.
| return (((long)TickCount()) * 1000) / 60; | |
| return (((long)TickCount()) * 60) / 1000; |
According to https://www.ioi-xd.net/os9docs/the_tickcount_function.html that function return's 60ths of a second and as we want 1/1000 this function seem to need the reversed calculation as suggested above
There was a problem hiding this comment.
TickCount()->millis is the tick count multiplied by 16.6~, while millis->TickCount() ticks is the milliseconds divided by 16.6~. Since we can't do floats it is multiplied by 1000/60, which might make it look confusing. The best way to think of it is the bigger number after the * is for converting TickCount()->millis, whereas the small number after the * is for millis->TickCount(). I'll add a comment to that
| { | ||
| PDC_mac_process_events(1); | ||
| PDC_check_for_blinking(); | ||
| } |
There was a problem hiding this comment.
Not sure, but shouldn't we sleep here to not eat cpu?
There was a problem hiding this comment.
PDC_mac_process_events() takes in a wait time for WaitNextEvent in TickCount() ticks, which will handle that. I will comment that XD
| PDC_LOG(("PDC_napms() - called: ms=%d\n", ms)); | ||
| if (ms <= 0) | ||
| return; | ||
| waitticks = ((long)ms * 60L + 999L) / 1000L; |
There was a problem hiding this comment.
please add a comment on this calculation,with the second glance I still don't get it... but shouldn't we use PDC_millisecs here in the first place?
There was a problem hiding this comment.
It is the millisecond->TickCount() ticks calculation, the reverse of what you are thinking of I think? The function takes in milliseconds and we add just under a tick to it (a sort of ciel()) to round it, converting it to ticks for the event wait loop below that. Yes, I should comment that, and if there is a better way let me know. I did do it without the rounding at first but it didn't work quite right....
There was a problem hiding this comment.
Do we need the CodeWarrior Project here? I think that's not part of the port, but of the editing, no?
As we don't include that for other ports, I think this should be dropped.
There was a problem hiding this comment.
That's the windows codewarrior 6 project file for cross compiling to mac. Useful for me, probably not for most others so I'll remove it
There was a problem hiding this comment.
same for the above, seems to be editor tooling, not related to the port
There was a problem hiding this comment.
That's the native macintosh codewarrior project file. I COULD remove it but it is the only way to natively compile and debug it XD, otherwise you'd have to use Retro68 cross compilation.
There was a problem hiding this comment.
We can't cross-compile with CMake, can we? If not why should we have the CMake stuff in there?
... if you already have info about local and/or cross-compilation some hints (may be one or two sentences) int he README would be nice.
There was a problem hiding this comment.
Yes we can, with Retro68 (https://github.com/autc04/Retro68) (It's a GCC version that cross compiles to macintosh). A lot of mac enthusiasts use it since it works with the CMake toolchain and they can use modern c/c++ with it and the compilation is completely native on Linux/FreeBSD. On windows you would set it up in WSL and compile it with Visual Studio or similar.
There was a problem hiding this comment.
Only used for the editor (see next file's comments), no?
Then should be removed.
There was a problem hiding this comment.
Its the resource file and tells finder how much memory the app needs and stuff. Its used with the demos, I think I could remove it? Old mac system OSes can get really picky if you dont have a SIZE resource sometimes
There was a problem hiding this comment.
Thanks for testing, if the demos don't work without the .r file a minimal note in the README about that is fine.
There was a problem hiding this comment.
if this needs to use Mac linebreaks (similar for the others) then please ensure that via git attributes, allowing the repo to stay in utf8
There was a problem hiding this comment.
It doesn't - thats just an artifact of editing it on mac. I'll have it match the other files
|
Thanks for the response. Give me a bit to address them. I made sure all the demos and my own couple apps ran decently. I THINK I ran the tests (its been a few weeks and ive worked on a million projects since then), I'll doubly make sure for that. |
Fix mac newlines. Add README.md to mac. Add some comments. Remove forced Carbon compilation from CMake; leftover artifact from debugging
Reserve PDC_MAC_GROW_MARGIN so grow box can show up Queue a mouse key event for each real mouse move so "getmouse failed" isn't an issue Don't ignore click-wait timing Don't produce an extra released event on mouse up after dragging window or using menu Host of text displaying unicode fixes so all macroman characters can show; only those supported by the font can show though FUNCTION_KEY_ABORT now works correctly Only event poll once per tick for performance reasons Retro68 fix for getch() CMake enhancements for Retro68 demos/testcurs.c get_wstr cast fix as it returns an empty string on Retro68 PPC
|
Thanks for doing this... looks interesting! I've not really dug into it much. One quick comment, while I'm thinking of it : will overflow for (1) if With this, you're good until |
… of black when resizing from smaller to larger Prevent overflow in ticks<->milliseconds calculation Native codewarrior edge case fix for its erase enum in system headers Rejigger native codewarrior project a tad to be compressed so resources carry over
|
Thanks for the comment Bill you are right. I moved to shifts on one of them because GCC refused to optimize it but other than that it looks good. I added some stuff to the readme about Retro68 - I don't think I want to add too much more because it is verging on being a full blown tutorial for it and there are plenty of guides out there. Everyone's feedback should be addressed, and I fixed all the edge cases from the tests I saw. If there's anything I missed let me know. |
I used the wingui port as a reference. There were only a couple tiny changes outside the mac directory where my port is. Attempted to be System 6-safe; it works on System 7 and is carbonlib 1.0 compatible so it works on early OSX as well. Cmake included as well as Codewarrior project files for mac and windows.