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

Do not use SDL render API for certain drawing operations #4704

Merged
merged 1 commit into from Jan 14, 2020

Conversation

irydacea
Copy link
Member

With the introduction of batched rendering in SDL 2.0.10, the behaviour of the render API appears to have changed so that certain operations forcibly queue a renderer clip rectangle set operation, using a default rectangle if necessary. The command queue does not clean after itself since it appears that the operating assumption is that you either use the renderer API or you don't, and Wesnoth performs drawing operations both with and without it in a few places.

With commit 4cbd652, our drawing primitives were changed ("refactored") so that the render API is forcibly used by them. When combined with contexts that use the clip_rect_setter object, things get weird since the clipping rectangle is reset behind the code's back.

As I see it, there's two solutions to this:

  1. Make clip_rect_setter use SDL_RenderSetClipRect(). The problem with this is that there are at least 3-4 places using clip_rect_setter on a target that isn't the screen framebuffer. Finding out whether the target surface is the screen seems like an inconvenient chore.
  2. Don't use the render API ever.
  3. Keep using the render API (a lot of other things do use it, such as the GUI2 canvas implementation) and change the few contexts where we use clip_rect_setter together with drawing primitives to call the SDL_Surface drawing primitives directly instead of using the render API.

This patch goes with option 3 since it seems the least intrusive. While this fixes the two known cases of bug #4510 as of this writing (help browser and minimap outline), I am not entirely sure if there are any other users of clip_rect_setter hiding drawing primitive calls somewhere underneath.

Also added relevant source comments in case someone decides to refactor the involved code and break it again. It's especially necessary in the minimap's case since we need to draw a grand total of 4 different rectangles at once.

Fixes #4510.

@irydacea irydacea added the Graphics Issues that involve the graphics engine or assets. label Jan 13, 2020
With the introduction of batched rendering in SDL 2.0.10, the behaviour
of the render API appears to have changed so that certain operations
forcibly queue a renderer clip rectangle set operation, using a default
rectangle if necessary. The command queue does not clean after itself
since it appears that the operating assumption is that you either use
the renderer API or you don't, and Wesnoth performs drawing operations
both with and without it in a few places.

With commit 4cbd652, our drawing
primitives were changed ("refactored") so that the render API is
forcibly used by them. When combined with contexts that use the
clip_rect_setter object, things get weird since the clipping rectangle
is reset behind the code's back.

As I see it, there's two solutions to this:

 1. Make clip_rect_setter use SDL_RenderSetClipRect(). The problem with
    this is that there are at least 3-4 places using clip_rect_setter on
    a target that isn't the screen framebuffer. Finding out whether the
    target surface *is* the screen seems like an inconvenient chore.

 2. Don't use the render API ever.

 3. Keep using the render API (a lot of other things do use it, such as
    the GUI2 canvas implementation) and change the few contexts where we
    use clip_rect_setter together with drawing primitives to call the
    SDL_Surface drawing primitives directly instead of using the render
    API.

This patch goes with option 3 since it seems the least intrusive. While
this fixes the two known cases of bug wesnoth#4510 as of this writing (help
browser and minimap outline), I am not entirely sure if there are any
other users of clip_rect_setter hiding drawing primitive calls somewhere
underneath.

Also added relevant source comments in case someone decides to refactor
the involved code and break it again. It's especially necessary in the
minimap's case since we need to draw a grand total of 4 different
rectangles at once.

Fixes wesnoth#4510.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graphics Issues that involve the graphics engine or assets.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wesnoth does not render well with SDL 2.0.10
2 participants