Skip to content

Split MyOpenGLView into two - #730

Open
mcarans wants to merge 7 commits into
OoliteProject:masterfrom
mcarans:split_myopenglview_gl
Open

Split MyOpenGLView into two #730
mcarans wants to merge 7 commits into
OoliteProject:masterfrom
mcarans:split_myopenglview_gl

Conversation

@mcarans

@mcarans mcarans commented Aug 30, 2026

Copy link
Copy Markdown
Contributor
  • MyOpenGLView -> MyOpenGLView and MyOpenGLView+Input
  • isOutputDisplayHDREnabled in MyOpenGLView uses SDL3 instead of Win32 (can be used by Linux in future)
  • Remove dead code from MyOpenGLView

phkb
phkb previously approved these changes Aug 31, 2026

@phkb phkb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good from my end.

@mcarans

mcarans commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

@oocube @LW-archlinux are either of you able to test this?

@LW-archlinux

Copy link
Copy Markdown

The commit messages don't mention the reason for the actions.

  • What was the dead code for and why do you think it's no longer needed ?
  • I understand why you plan to use SDL3 to implement HDR and agree that route gives the best chance for cross-platform HDR support in oolite, but others may not. Please elaborate.
  • What issues are solved by splitting MyOpenGLview ?

@mcarans

mcarans commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

@LW-archlinux The dead code in MyOpenGLView was found by using a Python script to look for dead code candidates (ie. code that isn't called from anywhere else). I described it here. I took just the MyOpenGLView ones from that list and checked them manually for usages:


-[bounds]
    /home/mcarans/Code/OoliteRelated/oolite_fixes/src/SDL/MyOpenGLView.m:757
    STATUS: CANDIDATE

-[indexOfCurrentSize]
    /home/mcarans/Code/OoliteRelated/oolite_fixes/src/SDL/MyOpenGLView.m:857
    STATUS: CANDIDATE

-[pngSaveSurface:withSurface:]
    /home/mcarans/Code/OoliteRelated/oolite_fixes/src/SDL/MyOpenGLView.m:1553
    STATUS: CANDIDATE

-[translateKeyCode:]
    /home/mcarans/Code/OoliteRelated/oolite_fixes/src/SDL/MyOpenGLView.m:1566
    STATUS: CANDIDATE

-[isAlphabetKeyDown]
    /home/mcarans/Code/OoliteRelated/oolite_fixes/src/SDL/MyOpenGLView.m:1694
    STATUS: CANDIDATE

-[isCommandDown]
    /home/mcarans/Code/OoliteRelated/oolite_fixes/src/SDL/MyOpenGLView.m:1742
    STATUS: CANDIDATE

I was conservative so I did not remove isAlphabetKeyDown or isCommandDown as it seemed possible they could be used in future. The other functions were not called from anywhere else so I removed those 4 functions.

HDR was implemented in Win32 calls because SDL1.2 did not have HDR support. SDL3 does have HDR support. The function isOutputDisplayHDREnabled is used to check if HDR is possible. That 150 lines of Win32 code can now be done in just a few lines of cross platform SDL3 code. Generally where SDL3 equivalents now exist for Win32 code, it is logical and sensible to use it.

Many source files in Oolite need splitting. It is very challenging to understand source files of 1000 lines+ of code. Part of the reason for the complexity was probably trying to bolt SDL into a Cocoa first design + code rot as features and hacks were spliced in to add new functionality or solve specific issues without refactoring the code to make it readable and understandable. This split (as well as deleting dead code and replacing OS custom code with SDL3) is a small step towards making the code a bit easier to follow.

@LW-archlinux

Copy link
Copy Markdown

It looks like the changes to mk.sh can't apply cleanly to master 942f518 , you may have to rebase this.

==> Starting prepare()...
patching file src/SDL/MyOpenGLView.h
patching file src/SDL/MyOpenGLView.m
patching file mk.sh
Reversed (or previously applied) patch detected!  Skipping patch.
11 out of 11 hunks ignored -- saving rejects to file mk.sh.rej
patching file src/SDL/MyOpenGLView+Input.m
patching file src/SDL/MyOpenGLView.m
patching file src/SDL/meson.build
patching file src/SDL/MyOpenGLView.m
patching file src/SDL/MyOpenGLView+Input.h
patching file src/SDL/MyOpenGLView+Input.m
patching file src/SDL/MyOpenGLView.h
patching file src/SDL/MyOpenGLView.m
patching file mk.sh
Reversed (or previously applied) patch detected!  Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file mk.sh.rej
patching file mk.sh

@mcarans

mcarans commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

@LW-archlinux I rebased. How is it now?

@LW-archlinux

Copy link
Copy Markdown

Still fails, the commits that change mk.sh probably should not be present.
I'll add the separate commits individually to identify if those are indeed the problem .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants