bmx-ng / sdl.mod

SDL backend for BlitzMax
7 stars 6 forks source link

gl2sdlmax2d: DrawRect() ignores ViewPort #6

Closed GWRon closed 6 years ago

GWRon commented 8 years ago

Setting the viewport on a gl2sdlmax2d-graphics-context seems to get ignored by DrawRect().

Bitmaps (TImage) handle viewport limitations (SetViewport(x,y,w,h)) correctly

GWRon commented 8 years ago

Appending a "drawText(...)" right after the drawRect results in a correct handling of the viewport in the drawrect() command. The drawText-text is not displayed (which is correct).

GWRon commented 8 years ago

That issue leads to interesting effects:

But when

Example: "demoapp" in the Dig-Framework when run with gl2sdl (so manually edit the code - base.util.graphicsmanager.bmx - or run it on android ;-)

GWRon commented 8 years ago

Sample - compare glmax2d versus gl2sdlmax2d:

SuperStrict
Framework Brl.StandardIO

Import brl.Graphics
?bmxng
Import sdl.gl2sdlmax2d
?not bmxng
Import Brl.glmax2D
?

'SetGraphicsDriver GL2Max2DDriver()
Graphics(800,600)

?Not android
Const KEY_BROWSER_BACK:Int = 0
?

Repeat
    Cls

    SetViewport(0,0, 100,100)
    SetColor 0,255,0
    DrawRect(50,50,100,100)
    SetColor 255,0,0
    DrawRect(100,100,100,100)
    SetColor 255,255,255
    SetViewport(0,0, GraphicsWidth(),GraphicsHeight())

    Flip 1
Until KeyHit(KEY_ESCAPE) Or KeyHit(KEY_BROWSER_BACK)

Result with glmax2d (how it should behave at all): glmax2d

Result with gl2sdlmax2d: gl2sdlmax2d

GWRon commented 8 years ago

I assume it has something to do with "Flush()".

When replacing

    SetViewport(0,0, GraphicsWidth(),GraphicsHeight())
    Flip 1

with

    Flip 1
    SetViewport(0,0, GraphicsWidth(),GraphicsHeight())

(or place that "resetting"-SetViewport right after the Cls) results in the correct behaviour

GWRon commented 8 years ago

Ok the problem is basically this:

So the solution would be to enqueue these immediate-commands too

GWRon commented 8 years ago

Ok, found another solution: I Flush() on Cls() and on SetViewport() (before doing the actual work) - and it seems to fix the issues.

Edit: I appended 2 new primitives: VIEWPORT and CLS - and use FlushTest(primitive) now (so a double "cls" only flushes once)

For "setBlend()" we do not need this adjustment, as the latest blend-command is processed with the next drawn primitive (so rect,oval,..., image, cls, viewport)

GWRon commented 8 years ago

Next to this approach, it should be possible to mark by "flag" if Cls() was called, or SetViewport(). Within FlushTest() the test fails then if one of the flags is active. So as soon as something new would get batched, the Flush()-call would happen. This is similar to a "dirtyRects"-approach (marking things for "re-doing").

Will provide 2 potential patches - feel free to accept the one you prefer.

GWRon commented 8 years ago

Hmm, Ok, the second approach must be a mix of the old and the new style. SetViewPort() is similar to SetBlend() or SetColor() - so it could use the same style: it could get checked and if it differs to the last one, it should call Flush(). As comparing 4 numbers (x,y,w,h) is more cpu-hungry than comparing a single one (viewport_changed = True) I would use this option.

For Cls() I think this is similar to drawing a big rect, so I would handle it similar.

GWRon commented 8 years ago

Seems the alternative approach cannot get used that easily, because within Flush() checks of the primitive_id are done. This does not change on a SetViewport()-call - which leads to uncontrollable behaviour (processed shaders).

So I would prefer to use the approach in my first Pull Request: utilizing Cls and SetViewPort as individual "primitives". -> sample code above and "demoapp" look as they should.

woollybah commented 6 years ago

This appears to be working now, as per the PR.