Skip to content
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

Use SDL for replay. #88

Merged
merged 1 commit into from
Jun 19, 2014
Merged

Use SDL for replay. #88

merged 1 commit into from
Jun 19, 2014

Conversation

armyofzin
Copy link

This change uses the SDL to create a window and handle events for the trace replayer (command line and editor). This change has a pervasive impact so any code reviews would be appreciated.

@mikesart
Copy link
Contributor

I'm pretty sure we don't care about X11 anything once we get the SDL paths working. That's our current line of thinking at least...

@lawlove
Copy link
Contributor

lawlove commented Jun 18, 2014

OK good - just thought I'd check.

On Wed, Jun 18, 2014 at 2:02 PM, Michael Sartain notifications@github.com
wrote:

I'm pretty sure we don't care about X11 anything once we get the SDL paths
working. That's our current line of thinking at least...


Reply to this email directly or view it on GitHub
#88 (comment).

@armyofzin
Copy link
Author

Lawrence is right. There's a couple spots where I can just remove the X11 code instead of ifdef'ing it out. Working on that now. Should have it done in a few minutes.

The only caveat is the glxUseXfont stuff that I'm not sure how to work around with the SDL. I need to look at that more carefully. I left it as a TODO for now... hopefully that doesn't ruin anybody's day.

@mikesart
Copy link
Contributor

If it's something not in SDL that makes sense to get into SDL, then we should try to do an official SDL patch. Checking with Sam / Ryan makes the most sense on those.

Otherwise we'll have to use SDL_GetWindowWMInfo() and have some code in there to handle the different platforms - something like I did in glxspheres.c. Maybe we should get this type of thing into vogl_port though?

@armyofzin
Copy link
Author

Talk directly to the device through the SDL?

@mikesart
Copy link
Contributor

Talk directly to the device through the SDL?

Yea - I'm talking about implementing vogl_replay_window::is_direct() and the font cache stuff by either getting a patch into SDL or putting the functionality into our vogl_port using SDL_GetWIndowWMInfo() and the native handles. Option #2 probably makes the most sense right now?

@armyofzin
Copy link
Author

Alright. Do you want me to get that in this branch? Also I removed more X11/GLX code that Lawrence noticed.

@mikesart
Copy link
Contributor

Do you want me to get that in this branch?

Would be nice to have a TODO comment in the is_direct() call, but otherwise no - doing it in another checkin is fine. Thanks Jeremy!

@mikesart
Copy link
Contributor

I'm also getting a ton of warnings on Linux when building with Clang 3.4.1 (in the vogl chroot). Are you seeing this as well?

@armyofzin
Copy link
Author

If you're referring to all the SDL-Doxygen warnings, then yeah I see those too. I think it might be something special clang is doing. It's on my todo list to silence those. It's coming directly from the SDL source.

@armyofzin
Copy link
Author

Turns out Clang can parse Doxygen-style comments with the -Wdocumentation flag, which is damn cool. Using -Wno-documentation turns it off, but I don't think we want to do that? Ideally we just get this fixed in SDL.

@mikesart mikesart merged commit fa1c2c4 into ValveSoftware:master Jun 19, 2014
#else
# error "Need to handle *GetCurrentContext for this platform."
#error "Need to handle *GetCurrentContext for this platform."
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding a line-attached comment for testing purposes.

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.

5 participants