Slider rendering to offscreen buffer

[Bigpet]

OK, so this is a little bit hacky and I haven't programmed Java in years so take this pull-requests more as a starting point for a discussion.

This is rendering sliders to an offscreen buffer and then rendering the offscreen buffer to the screen.

The obvious sticking point is "Image.getGraphics()" which requires PBO or FBO support and uses more memory.

Also, Slick2D is what makes the initializing of the buffers slow (the "Image.getGraphics()") since it seems to copy the empty image to the GPU, even though the very thing I do with it is to clear it. Additionally the weird black lines in the diagonal seem to be an artifact from the way Slick2D renders to the offscreen buffer. Also, the lower alpha is also a result of the lack of control over the blending in Slick2D (and isn't as noticable with other themes).

So my question is, would you be opposed to actually do the rendering with LWJGL directly to get better control instead of relying on Slick2Ds rendering?

This commit

Before

[itdelatrisu]

Thanks for working on this! I actually achieved something quite similar to this when I tried a few months ago, but didn't end up pushing it for the reasons you listed -- specifically, the lower alpha levels and the lack of a "solid" color. On the other hand, I do like your borders a lot, and they look far better than the current implementation.

I'm totally fine with using pure LWJGL to render sliders -- it's definitely the better solution, but I just won't really be able to maintain the code (as my knowledge of OpenGL is very minimal XD). I don't know if you've seen this, but mm201 put up the source code for the default osu! slider rendering type, and ideally it'd be best if someone could port that to opsu!. Do you want to take a look?

[Bigpet]

Oh, I see, did not know about that thread. I'm gonna take a look if I can't just implement that with 1D textures, that makes a lot of sense.

Also, the lines are from the GL_POLYGON_SMOOTH that Slick2Ds "Antialiasing" enables, if I turn that off the diagonal lines disappear.

Btw. I know it's off-topic but do you have an IRC channel or something for this project to hang out in?

[itdelatrisu]

Yeah, Slick's antialiasing has major problems (it didn't work on almost every machine I've tried it on)... Let me know about any progress you make on this, and I'll be glad to help out if I can!

I don't have an IRC channel or anything, but feel free to contact me via email or on the osu! forums; if you want to chat, I can meet you in IRC (or wherever) as long as I'm free. ^.^

[Bigpet]

Well using the depth buffer does indeed work rather well

I just have to get the 1D texture mapping to work right

[Bigpet]

alright got the concept working

Now I just need to clean it up a lot or better yet abstract the whole slick rendering. Because having slick mixed with raw opengl feels really weird (their textures are really strange, slick seems to compensate for npot and that means my 640px test texture went from 0.0 to 0.625 instead of 0.0 to 1.0)

[itdelatrisu]

Wow, that looks fantastic! It's really impressive... o_o How's the performance compared to before?

Since osu! does support both slider rendering modes -- peppysliders (your original offscreen rendering) and mmsliders (the new kind) -- it'd be great if the rendering could be separated somehow (new classes? or whatever...) to allow switching between the two, though I personally don't care if it's too much of a hassle. What do you think?

[Bigpet]

I would like to keep the old method around as an option if possible (since the new one will have weird artifacts if I try to work around not using FBOs and FBO support with old Intel integrated GPUs in Windows is terrible)

As for performance, I can't notice any difference but I may need to tweak things. But as a side-note introducing an animation like osu! has (building the slider from beginning to end over multiple frames) may actually increase performance (since it can gradually build the slider image into the fbo).

I mean there's not much happening, it's just drawing a few 30-tri cones while building the slider. Once the full slider is drawn into the FBO performance should not be an issue (since from then on it's just texture-lookups)

(Also this pull-request is not ready yet and I did not push my latest tests that are represented in the screenshots to github, I'll rebase it and make a comment when I think merging would be advisable)

edit: since cleanup might take a while I'll be uploading my work in progress stuff in the meantime, just don't merge it yet

[itdelatrisu]

Just checking in -- how's progress on this?

[Bigpet]

Haven't gotten around to making much more than I pushed to the feature branch. But I'm hoping to take the time to do it this month.

[itdelatrisu]

Okay, sounds good! Thanks for letting me know!

[itdelatrisu]

Tried playing a beatmap, got this error immediately before a slider was supposed to appear:

# A fatal error has been detected by the Java Runtime Environment:
#
#  EXCEPTION_ACCESS_VIOLATION (0xc0000005) at pc=0x000000001a4ad6a4, pid=8124, tid=4648
#
# JRE version: Java(TM) SE Runtime Environment (8.0_31-b13) (build 1.8.0_31-b13)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (25.31-b07 mixed mode windows-amd64 compressed oops)
# Problematic frame:
# C  [ig75icd64.dll+0x7d6a4]

(...)

Stack: [0x00000000025c0000,0x00000000026c0000],  sp=0x00000000026bcc20,  free space=1011k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
C  [ig75icd64.dll+0x7d6a4]
C  [ig75icd64.dll+0x1659c5]
C  [ig75icd64.dll+0x20dfaa]
C  [ig75icd64.dll+0x58a00]
C  [ig75icd64.dll+0x257f75]
C  [ig75icd64.dll+0x7c992]
C  [ig75icd64.dll+0x153210]
C  0x00000000028b5b74

Java frames: (J=compiled Java code, j=interpreted, Vv=VM code)
j  org.lwjgl.opengl.GL11.nglDrawArrays(IIIJ)V+0
j  org.lwjgl.opengl.GL11.glDrawArrays(III)V+20
j  itdelatrisu.opsu.objects.curves.LinearBezier.draw()V+667
j  itdelatrisu.opsu.objects.Slider.draw(Lorg/newdawn/slick/Graphics;I)V+209
j  itdelatrisu.opsu.states.Game.drawHitObjects(Lorg/newdawn/slick/Graphics;I)V+614
j  itdelatrisu.opsu.states.Game.render(Lorg/newdawn/slick/GameContainer;Lorg/newdawn/slick/state/StateBasedGame;Lorg/newdawn/slick/Graphics;)V+1989
j  org.newdawn.slick.state.StateBasedGame.render(Lorg/newdawn/slick/GameContainer;Lorg/newdawn/slick/Graphics;)V+54
j  org.newdawn.slick.GameContainer.updateAndRender(I)V+307
j  itdelatrisu.opsu.Container.gameLoop()V+33
j  itdelatrisu.opsu.Container.start()V+17
j  itdelatrisu.opsu.Opsu.main([Ljava/lang/String;)V+263
v  ~StubRoutines::call_stub

I can provide the full dump if you want, but the rest doesn't look too helpful.

[Lemmmy]

I've had similar errors with other LWJGL games and it was something on the graphics card's end. I'm just cloning and testing now.

[Bigpet]

Nah, the commit is broken at the Moment. I have a bug to iron out.

[Lemmmy]

Ah ok

[Bigpet]

I was in the middle of converting the immediate mode rendering into vertex-buffer based rendering.

It's just that and then the tone-remapping in the shader and I should be done. Currently battling with the coordinate system a little, but it shouldn't take too long

[itdelatrisu]

Okay, no rush! I was just a bit misled by your "almost done" commit. XD

[Bigpet]

Alright, now you can try it out. This took longer than I thought (had a very curious bug with lwjgl and not being allowed to use buffers from ByteBuffer.allocateDirect() which I could only debug with the help of apitrace)

I'm going to rebase now before putting on the final touches.

What's missing is to adjust middle accent color (I'm probably going to need to convert the color to HSV and shift the hue)
and less stupid rendering (this code just screams for instanced rendering)

[Lemmmy]

ahmg i just cloned stop making commits ._.

[Bigpet]

no, let me finish before you merge, I'll let you know when this is ready.

I will do the rebasing on top of 0.9 myself if that's what you were planning

[Lemmmy]

nah i was just testing it's okay
looking damn fine mate!

couldn't detect any performance differences but then again i do have a gtx 760....

[itdelatrisu]

It does look pretty good! Sizing issues though (1440x900):

[itdelatrisu]

What's the purpose of these as opposed to container.getWidth() or Display.getWidth()?

[itdelatrisu]

Scaling works perfectly now!

Something I've noticed is that memory usage goes up for every slider that gets drawn -- up to around 200mb for a random beatmap I tried (before getting garbage collected afterwards). That's pretty substantial; is there any way around this?

[Bigpet]

I don't actually see where that size could come from.

The allocations I make are just the FBOs ( 1440 * 900 * (4+4) *4 = 39MB ) which should be on the GPU memory actually (and don't get free'd after a song is over either), and the around 100k-500k for the vertex buffer when first drawing the slider (depending on how many curve points it has) which I will probably reduce (by a factor of 128) by using instancing. But the vertex buffers are generated and deleted on the first frame that a slider appears (deleted on the OpenGL side, I obviously have no control over the Java side).

I may try to reuse the ByteBuffer (that I have to allocate with lwjgl.BufferUtils otherwise lwjgl will just send OpenGL garbage) if lwjgl is keeping that thing alive for some reason.

The Netbeans memory profiling tools don't seem to be too helpful for some reason.

Here's some profiling runs I made, looks to be all in the margin of error (being the gc in this case) to me:

[Bigpet]

Hm, alright so fading out is fixed and I had to find another place to remove the cached mapping.

I think I found the best place now in GameData, but it presumes that GameData is never deleted while its referencing these curve hitobjects.

[itdelatrisu]

Hmm, memory usage seems fine now. I wasn't doing any actual profiling -- just looked at my task manager while running the program.

GameData shouldn't be changed during a single game, but may be changed between games and when viewing replays.

[Bigpet]

Alright I think that's it, should be alright to merge after you look through it one last time.

All the missing optimizations probably make more sense after some other changes. There may still be some visible artifacts especially on higher resolutions and certain curvatures you may see some slightly more opaque bands perpendicular to the curve direction.

[itdelatrisu]

This is what it looks like for me:

[Bigpet]

That confuses me greatly. Can you see if that still occurs with this branch: https://github.com/Bigpet/opsu/tree/slideradjust ( binary here ) ?

and please tell me your OS/GPU and if possible driver version

[itdelatrisu]

It might be a scaling issue. This is how it looks in 1024x768:

And at 1400x900 (the resolution of my previous post):

I'm running a Windows 8.1 laptop, display details here:

All the sliders seemed to work fine before the last few commits.

[Bigpet]

Ok for some reason GameImage.SLIDER_GRADIENT.getImage().getWidth() reports a wrong value and I have no idea why

[Bigpet]

Should be without that weird middle line now

[itdelatrisu]

Okay, it's good now. All that's left is the SliderBorder skin color, but that can be dealt with later. I'll go ahead and merge this now -- again, thank you so much for doing all of this!

[itdelatrisu]

There are a lot of these warnings logged (from FrameBufferCache):

Framebuffer cache was not large enough, possible resource leak.

What's going on? (Also, INITIAL_CACHE_SIZE isn't used -- not sure if that's related.)

[Bigpet]

Yeah, I guess I should remove that warning. The initial cache size was still from when I used the Slick framebufferobject wrapper which did some RAM->VRAM copy. But now that we don't actually do any useless RAM->VRAM transfer the startup cost of adding another fbo is not noticable.

Also, I can't get the beatmap parser to actually find the "SliderBorder" setting. Any hint there?

[itdelatrisu]

Pushed a follow-up: b6f208a (didn't see your last comment in time, so I didn't get rid of the warning yet)

SliderBorder is in skin.ini, and you can get the value with Options.getSkin().getSliderBorderColor().

[Bigpet]

@itdelatrisu yeah that works for skins, but I'm talking about the beatmap specific values

because I set a breakpoint here:

opsu/src/itdelatrisu/opsu/beatmap/BeatmapParser.java

Line 497 in 00a08f1

LinkedList<Color> colors = new LinkedList<Color>();

and nothing got hit

[itdelatrisu]

I don't think SliderBorder is given in .osu files. The slider border definitely changes color based on hitcircleoverlay, though (e.g. it's black in this map), but I don't have any idea how that's done. (The Combo[1-8] only determine the hit object colors, not the SliderBorder color. You probably didn't get any hits because everything's being read from the SQLite cache.)

[Bigpet]

Well I have it in 30232, 30688 and most noticeably 32510 but I'll take a look at the map you posted

[itdelatrisu]

Wait, you're right. I didn't see it at all. x-x Will fix asap...

[Bigpet]

@itdelatrisu oh, I get what the issue is. The beatmap DB makes the parsing skip. So I need that value in the Beatmap DB, right?

[itdelatrisu]

Added it to the parser/database in 447a0f3. Use beatmap.getSliderBorderColor() to get the color. Sorry about that. x.x