# Is 2 lines of push/pop code for each pre-draw-state too many?

I'm trying to simplify vector graphics management in XNA; currently by incorporating state preservation. 2X lines of push/pop code for X states feels like too many, and it just feels wrong to have 2 lines of code that look identical except for one being push() and the other being pop().

The goal is to eradicate this repetitiveness,and I hoped to do so by creating an interface in which a client can give class/struct refs in which he wants restored after the rendering calls.

Also note that many beginner-programmers will be using this, so forcing lambda expressions or other advanced C# features to be used in client code is not a good idea.

I attempted to accomplish my goal by using Daniel Earwicker's Ptr class:

    public class Ptr<T>
{
Func<T> getter;
Action<T> setter;

public Ptr(Func<T> g, Action<T> s)
{
getter = g;
setter = s;
}

public T Deref
{
get { return getter(); }
set { setter(value); }
}
}


an extension method:

        //doesn't work for structs since this is just syntatic sugar
public static Ptr<T> GetPtr <T> (this T obj) {
return new Ptr<T>( ()=> obj, v=> obj=v );
}


and a Push Function:

        //returns a Pop Action for later calling
public static Action Push <T> (ref T structure) where T: struct
{
T pushedValue = structure; //copies the struct data
Ptr<T> p = structure.GetPtr();

return new Action( ()=> {p.Deref = pushedValue;} );
}


However this doesn't work as stated in the code.

How might I accomplish my goal?

Example of code to be refactored:

    protected override void RenderLocally (GraphicsDevice device)
{
if (!(bool)isCompiled) {Compile();}

//TODO: make sure state settings don't implicitly delete any buffers/resources
RasterizerState oldRasterState = device.RasterizerState;
DepthFormat oldFormat = device.PresentationParameters.DepthStencilFormat;
DepthStencilState oldBufferState = device.DepthStencilState;
{
//Rendering code
}
device.RasterizerState = oldRasterState;
device.DepthStencilState = oldBufferState;
device.PresentationParameters.DepthStencilFormat = oldFormat;
}

• Is your problem that your trick with Ptr<T> doesn't work, or that you don't like having to call var pop = Push(ref something); ... pop();? – michael.bartnett Oct 7 '12 at 0:01
• @michael.bartnett It doesn't work because the extension implicitly gets a copy of the struct rather than its reference – Griffin Oct 7 '12 at 0:36
• Ah okay, my answer is mostly redundant then. Simply saying, "How might I accomplish my goal?" as a final wrap-up question was a little ambiguous, since you've got both a design and technical problem. – michael.bartnett Oct 7 '12 at 0:47
• Can you post an example of the code you're trying to improve? The bit with the 2X statements? – Andrew Russell Oct 7 '12 at 1:02
• @AndrewRussell I added an example – Griffin Oct 7 '12 at 1:40

Your Push<T> function isn't going to do what you want it to do. The Ptr<T> you create inside it is only accessible inside that function. In fact I would hope that the C# compiler would optimize away that call since it's effectively a noop.

The way to solve this problem is to make all of your possible state data classes instead of structs. Structs are a very unique thing in C#. They should absolutely only be immutable, data under every conceivable circumstance. Useful for quickly associating a bunch of values, or implementing numeric types like Vectors and special ID-type values.

The quick solution is to implement Push like this:

//returns a Pop Action for later calling
public static Action Push<T>(Ptr<T> pt) where T: struct
{
T pushedValue = pt.Deref; //copies the struct data
return new Action( ()=> {pt.Deref = pushedValue;} );
}


See example program here: http://pastie.org/4925186

Your clients must have knowledge of Ptr for it to work. In order to make the transition from value-type land to reference-type land in these Push/Pop functions, you have to pass around the Ptr.

Now that being said, you mentioned this should be for beginners. Why are you making them thinks about pointers in C#? Or maybe you aren't, but that's a consequence of this approach.

If you are actually going to be pushing and popping state, why not maintain an actual stack? (Or List that you use with Stack semantics). Or maybe just always copy the user data to a temp state before you render, leaving their own data untouched? Without seeing how your code would use this feature, it's tough for me to recommend a good approach.

I can see some beginners liking see both Push() and Pop() in their code though, since they can better understand then what's going on instead of assuming it's all magic.

• I originally had Push take a ref so that clients wouldn't have to know about the Ptr<> class. And the whole point of this is both to reduce repetitiveness, and to get the push/pop out of the client's virtual render function (where only rendering should be done). In order to maintain a stack or copy user data, I would still need a newbie-friendly way of telling the Drawable class where to get/set custom structs/classes that need preserving. – Griffin Oct 7 '12 at 2:02
• @Griffin How about a context object that wraps all of this then? It's pretty straightforward to edit the members of some sort of DrawableState object, then call PushState(currentDrawableState);. Then for convenience, you could give DrawableState a Clone() method that your push/pull methods would use, and the beginner had the option of using for more control. It's basic, but cleans up the code and lets you do some things automatically. – michael.bartnett Oct 7 '12 at 3:23

Much of OpenGL is basically a state machine. The important thing is that at each point where drawing occurs, that the correct state is set for all values; that there's no old, leftover invalid state left lying around.

Sample code typically ensures this by immediately turning everything off after using it, or through a push/pop system such as the one you mention in the question.

Real games typically implement a render state manager to sit in between the rendering engine and OpenGL, to optimise the state changes which are sent through to OpenGL. So instead of implementing push/pop, rendering code requests the specific features it wants from the state manager, tells the state manager to apply the changes, and then de-requests those features, and doesn't tell the state manager to apply to de-requests.

The code flow typically looks something like this:

m_state.SetBool( UsingVertexArray, true );
m_state.SetBool( UsingColorArray, true );
m_state.Flush();
ActuallyRender();
m_state.SetBool( UsingVertexArray, false );
m_state.SetBool( UsingColorArray, false );


The render state manager maintains its own internal state; it knows both the last setting it's told OpenGL about, and the most recently requested values. When "Flush()" (sometimes called "Apply()" or some other name) is called, it finds the settings with changed values, and only informs OpenGL about those changed values.

So if the next thing which renders after our sample code above turns "UsingVertexArray" and "UsingColorArray" back on again, m_state.Flush() does nothing, since there's been no change to the requested state compared against the last time "m_state.Flush()" was called.

This sort of approach has the advantage of being quite simple to understand, while still only sending real state changes to OpenGL; not spamming it with turning the same values on and off again over and over again, when everything actually wants them to be turned on.