HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Checking if a point is on screen

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
screencheckingpoint

Problem

I'm working on a tile-based game, and am looking to improve the efficiency of my code to slightly improve framerate. I call this method on every tile, every frame, so it is very performance critical. What optimizations can be made?

public bool OnScreen(Vector2 point) 
{
    Rectangle rect = this.Rectangle;
    return ((((rect.X <= point.X) && (point.X < (rect.X + rect.Width))) && (rect.Y <= point.Y)) && (point.Y < (rect.Y + rect.Height)));
}

public bool OnScreen(Vector2 topLeft, Vector2 size)
{
    // Check if each corner is inside of the camera rectangle
    return (
        OnScreen(topLeft) // Top-left
        || OnScreen(topLeft + new Vector2(0, size.Y)) // Bottom-left
        || OnScreen(topLeft + new Vector2(size.X, 0)) // Top-right
        || OnScreen(topLeft + size) // Bottom-right
    );
}


I am doing this so that I can pool the objects that actually render my tiles, since only a maximum of about 800 can be on screen at any one time. These methods are called as so:

// Assign renderers to any tiles in range of the camera.
foreach (var tile in _tiles)
{
    if (tile.IsInRange(camera))
    {
        if (tile.Renderer == null)
        {
            tile.Renderer = _pool.Get();
        }
    }
    else 
    {
        tile.Renderer = null; 
    }
}

Solution

At a glance, OnScreen looks like a method that would raise some Screen event, as On[EventName] is, by convention, the name we use for methods that raise an event. Now, those would obviously return void, and yours is returning a bool - I think I would have it like this:

public bool IsOnScreen(Vector2 point)


The return statement is pretty intense in terms of parentheses. Are they meant to improve readability?

return ((((rect.X <= point.X) && (point.X < (rect.X + rect.Width))) && (rect.Y <= point.Y)) && (point.Y < (rect.Y + rect.Height)));


Let's try something:

return rect.X <= point.X 
    && point.X < (rect.X + rect.Width) 
    && rect.Y <= point.Y 
    && point.Y < (rect.Y + rect.Height);


Much better isn't it?

Code Snippets

public bool IsOnScreen(Vector2 point)
return ((((rect.X <= point.X) && (point.X < (rect.X + rect.Width))) && (rect.Y <= point.Y)) && (point.Y < (rect.Y + rect.Height)));
return rect.X <= point.X 
    && point.X < (rect.X + rect.Width) 
    && rect.Y <= point.Y 
    && point.Y < (rect.Y + rect.Height);

Context

StackExchange Code Review Q#92096, answer score: 11

Revisions (0)

No revisions yet.