Jebbs / DSFML

DSFML is a D binding of SFML
Other
95 stars 23 forks source link

[2.3] Remaining features and cleanup #229

Closed aubade closed 7 years ago

aubade commented 8 years ago

Since you've mentioned the 2.3 release coming up, I thought it might be best to go over what remaining things you think are important to get in there prior to the release.

Some things I have heard discussed: -Convert View to a struct: This should be pretty easy. Looking at the C++ source, it doesn't contain anything that needs resource management. (i.e. no opengl structures)

-Use Templated draw() method instead of relying on the interface: The only difficulty with this is I don't believe templates and inheritance can work together (Thus, I believe RenderTarget will not be able to specify draw(T)(T drawable, RenderStates state); We might be able to work past this with some UFCS voodoo, but I'm not 100% on that.

-Look at lazy initializers (or other possible means of getting rid of static constructors and Default fields) I only see a couple of these things 1) I'm not sure we need to pass ContextSettings as a reference. If we pass by value we can just specify ContextSettings.init as the default parameter and be done with it. Most people aren't going to be creating windows every frame, so I think we can afford a little extra stack used for those calls.

2) I can see two ways of handling the emptyTexture and emptyShader objects in RenderStates: 2a) We can lazily initialize the structures. 2b) We can add nullchecks to any methods where we pass their sfPtrs on to C++ and just allow these to be set to null (I slightly favor this one as it feels slightly more robust to me, but either should work)

3) IpAddress' Localhost and Broadcast are created in their static constructor. For this one I'm tempted to suggest biting the bullet and replacing the C++ sfIpAddress_fromBytes and _fromInteger calls. These aren't complex algorithms and are unlikely to change (Unless SFML adds support for IPv6), so I'm tempted to just write D implementations that can be run at compile time. That failing, lazy initialization will also be a perfectly serviceable approach here.

-I don't really have the technical expertise to put together any Android/iOS tests (though I do have an android device I could help test with)

-BSD support is similar. I don't have any systems running FreeBSD, but in theory SFML's already done 99% of the heavy lifting for us here. DSFML has exactly three version(linux) blocks, which all pertain to X11 stuff and could theoretically be replaced with something along the lines of

version (Posix) 
{
    version (Android) {} else version (OSX) {} else
    {
        //Linux/BSD stuff goes here.
    }
}

Which isn't exactly elegant but I'm pretty sure those are the only two major Posix platforms that don't use X-based graphics systems.

-Finally my own personal white whale, I'd really for us to wrap the GLFlush call, since for anyone who wants to use a worker thread to procedurally generate textures it means they don't have to pull in an entire GL binding to their dependencies.

-Adding function decorations like pure, @safe, nothrow, @nogc, etc. This would be entirely doable but would probably be a couple days of drudgery to dig through all the methods.

I'm more than happy to do this work except where I otherwise specified, but I'd like a little bit of direction before I dive in and start filing pull requests.

Jebbs commented 8 years ago

Holy cow! Thanks for this!

One thing that isn't mentioned here that I was planning was to turn everything that doesn't/shouldn't use inheritance/poymorphism into a struct: #199.I was going to change them to be initialized lazily based on this idea: http://dpaste.com/3FH3W13 (but with copy constructor, etc). Might need to come up with a list though or decide if this is even a good idea or not. I figured that classes are probably over kill for a lot of the stuff like Clock, Texture, Image, Ftp, Http, and maybe more.

Another thing is unit tests need to start being a part of the build testing. There's some that tests that currently need user input, but those can be disabled and probably replace later. We know that keyboard/mouse input works.

There's also the usual packaging and website update, but I was also hoping to get to some tutorials and examples too.

There might be more to add to the list, but I can't think of anything off the top of my head. Perhaps I'll go through the issues and figure it out. Thanks for taking this stuff on. I'm still trying to figure out my schedule for the summer.

aubade commented 8 years ago

I'm still really hinky about using structs for references to C memory--That's how LuaD works and it sounds nice in theory, but in practice it's lead to a lot of headaches for me. Unless it's a singleton, we'd either be copying the C++ object every time we pass one to a function or assign it to a new variable, or we'd be essentially be reimplementing std.typecons.RefCounted which makes const and immutable a nightmare. Avoiding the GC is great, but std.experimental.allocators makes that really easy for a user to do themselves. (I've already started doing just that for asset management in my project)

Unit tests are always going to be kind of a tricky beast for us, since the obvious things are going to end up testing SFML. Does SFML itself have unit tests? I could investigate swiping some of theirs.

aubade commented 8 years ago

One idea that does occur, to help out users who want to do their own memory management, is that there are a few places where DSFML internally allocates things (Such as soundbuffer allocating a soundFileStream, Music's internal Mutex, and the Shapes' internal VertexArrays).

We could either switch these to explicit malloc-based allocation, or even set up static function pointers to allocate, reallocate, and free functions that default to malloc or GC, but that the user can assign to a custom allocator. This is a big enough change though, that I'd prefer to wait until after the 2.3 release to look at it.

(Internal GC allocations aren't inherently a problem, but they do make it so that if you allocate the containing object manually, you have to register/unregister it with GC.addRange, which is a bit of faff)

Alternately, for the cases where the allocation is of fixed size (i.e. not a dynamic array), we can even have an internal static ubyte array and use std.typecons.Emplace; This is roughly equivalent to having a non-reference C++ class stored internally.

aubade commented 8 years ago

Back to 2.3; I'm working on the View struct; Passing it to C++ is a bit heavy (it's a total of nine float values),

but some of the things that need views (such as the various map-window-to-world coordinate methods) are relatively simple logic that I could just reimplement in D and save us some very long function signatures in DSFMLC. Does that sound like a worthwhile approach?

EDIT I just implemented this in my local tree, and bringing the code into D? Also makes it so there only needs to be one copy of the code; interfaces do allow final methods to be defined (Or if we want them overridable it's easy to switch them to UFCS)

aubade commented 8 years ago

Just to warn, I just had a big hard drive wipe. I didn't lose any significant progress, but it'll be a couple days before I'm back up to speed.

aubade commented 8 years ago

I've been thinking about unit tests a little bit too; For some C++-referencing structures, we can make sure all the property passers are working correctly; e.g.

auto win = new RenderWindow(VideoMode(100, 100, 32));
assert((RenderWindow.position = Vector2i(100, -100) == Vector2i(100, -100));
assert(RenderWindow.position == Vector2i(100, -100));

For drawing, though we can take the added step of rendering things to a texture, dumping the texture to an image, and comparing them against a known reference.

auto shape = new CircleShape( 100);

//Preferrably set a number of properties on the shape to put as much stress
//on the drawing code as possible.

rendertexture.draw(shape);

auto reference = new Image();
assert(reference.loadFromFile("circlereference.png"));

assert(rendertexture.copyToImage.getPixelArray() == reference.getPixelArray()); 

This still relies on there being a working opengl and dsfmlc implementation on the testing machine, but it IS completely noninteractive and provided Mesa's software OpenGL can render to texture, could work on Travis-CI.

For just about anything else though, if we want to seriously unit test we're going to have to mock DSFMLC; essentially make fake versions of all those C++ functions that either return pre-defined results (such as a series of fake events or procedurally generated audio, such as a sine wave), or check their input against a known reference.

The only issue with this is that it will make it impossible to do interactive and noninteractive/rendertexture reference tests in the same compile. The benefit is that unit testing could be done without having to link DSFMLC

aubade commented 8 years ago

I've starting some example mocks on the FTP module, since it didn't have any tests yet and it's easy to illustrate the process:

https://github.com/aubade/DSFML/blob/2.3-mocktests/src/dsfml/network/ftp.d

It's kind of manual at the moment; there are mocking frameworks for D but none of them are targetted at our kind of use case.

Jebbs commented 8 years ago

Thanks for all the work you've been putting into this. Tomorrow is my last final for the term so I'll be able to start reviewing your stuff. It should go pretty quick though. There usually isn't much of your stuff that I would change.

aubade commented 8 years ago

Fantastic. Good luck on your finals!

Jebbs commented 8 years ago

I know it has been a while, but I'm finally getting back into this. I didn't have as much time over the summer as I thought. Turns out garbage collectors are hard to work on.

I'm curious on something though. Since 2.4 was released, should we try to change gears and catch up to that or just focus on 2.3 for the time being?

aubade commented 8 years ago

Hrm; checking the changelog, there may be some CMake changes; I used your work on those for 2.3 so I have no idea how hard that will be.

As far as API goes, the only really significant thing, i think, is going to be the addition of Geometry Shaders. My instinct is we can probably get the 2.4 changes in before release.

Jebbs commented 7 years ago

I'm going to go ahead and close this.

Everything discussed in here either has its own issue on the 2.4 milestone, is already merged into the 2.4 branch, or has been removed from the 2.4 todo list.

Thanks for all your hard work towards it! You've been a big help.