Skip to content
Snippets Groups Projects

Screenshot issue fixes

Merged Gordon Acocella requested to merge gordaxx727/t-engine4:screenshot-issues into master
  1. Fix save dialog appearing in savefile screenshot.
    Don't swap buffers when rendering for screenshots, since glReadPixels reads the back buffer.

  2. Fix positioning for savefile screenshot.
    Needed to flip the Y-coordinate due to coordinate system differences between SDL and OpenGL.

  3. Fix to not gamma adjust pixels of savefile screenshots when using shaders.
    Savefile screenshots looked to bright on the load game dialog because the pixels were saved with the gamma already applied. The gamma got applied again when displayed.

    Note: There is a lingering gamma adjustment made in game/modules/tome/class/Game.lua with the comment "Reset gamma setting, something somewhere is disrupting it, this is a stop gap solution". I left this adjustment in when not doing a savefile screenshot, although I don't see why it's necessary.

  4. Simplify SDL gamma calculation and restore default gamma on exit.
    When testing with shaders off, the gamma for the entire display would be changed and not restored on exit. Fixed that and also simplified the calculation to just use SDL_SetWindowBrightness, which calculates a simple gamma ramp behind the scenes.

  5. Fix to gamma adjust pixels of user screenshots when not using shaders.
    Unlike savefile screenshots, user screenshots are meant to be displayed outside the game. They should have the gamma applied to the pixels so the picture will reflect what the game looked like when the screenshot was taken. When shaders are used for gamma, this happens by default. When shaders are not used, the gamma adjustment must be made to the pixel values during the save.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • This looks nice! Thanks! Only problem I can see is that if the current game module doesnt sport a :setFullscreenShaderGamma() method then redraw_now() will explode :) Either check for it or define a dummy in engine.Game

  • Author Contributor

    Thanks, I'll try to fix it tonight after work.

    I guess I'm still a bit confused as to the class structure of a game. My intent was that by putting this routine in "game/engines/default/engine/Game.lua" other games would inherit this method (ToME does seem to inherit from this). I guess I'll have to go back and see where engine.Game gets defined.

    I've also been wondering how "game/engines/default/modules/boot/class/Game.lua" fits into things. Is there any existing document describing how the pieces of a game fit together at a very high level?

  • Awww fuck I'm sorry it's my mistake actually, I misread the diff :/ you did put it in the right place :) As for clarity anyway: When I refer to things like engine.SomeClass it does mean game/engines/default/engine/SomeClass.lua When I refer to things like mod.SomeClass it does mean game/modules/tome/class/SomeClass.lua OR game/engines/default/modules/boot/class/SomeClass.lua, depending on context The boot module is a module like any others, jsut not in the same location

  • I can't test this without a binary with it and definitely have no idea what its doing, but does this fix the issue where TOME will change desktop gamma in general not just fail to restore it on exit? As is even if TOME is minimized I have to constantly keep my video settings open and change it it back everytime I maximize TOME. It'd be really nice to have a way to just not let TOME mess with gamma outside of itself ever

  • Author Contributor

    I believe the issue you're talking about is caused by the way SDL handles gamma, so I don't think my changes will have any effect on this. Which platform and window manager, if applicable, are you using? If I can reproduce the issue I may be able to find a fix for it.

  • DarkGod Status changed to merged

    Status changed to merged

  • DarkGod mentioned in commit f036030a

    mentioned in commit f036030a

Please register or sign in to reply
Loading