Skip to content

Commit

Permalink
Ticket #101 - first pass
Browse files Browse the repository at this point in the history 
- Add `draw time` to debug panel
- Add dirty rects in reverse order (ensures debug panel is drawn last)
- Do not use `drawManage.makeAllDirty()` when `me.sys.dirtyRegion = true`
- Add rectangle information to TMX layers
- Small optimization in `TMXRenderer`
- Support `Infinity` dimensions in `me.Rect`
  • Loading branch information
Jason Oster committed Sep 10, 2012
1 parent 0f883ea commit 3b005e7
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 16 deletions.
41 changes: 31 additions & 10 deletions src/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,8 @@ var me = me || {};
// list of object to redraw
// only valid for visible and update object
var dirtyObjects = [];

var drawTime = 0;

var drawCount = 0;

Expand Down Expand Up @@ -916,9 +918,9 @@ var me = me || {};
if (oldRect) {
// merge both rect, and add it to the list
// directly pass object, since anyway it inherits from rect
dirtyRects.push(oldRect.union(obj));
dirtyRects.unshift(oldRect.union(obj));
} else if (obj.getRect) {
dirtyRects.push(obj.getRect());
dirtyRects.unshift(obj.getRect());
}
}
}
Expand Down Expand Up @@ -971,18 +973,26 @@ var me = me || {};
return drawCount;
};

/**
* return the time when drawing started
*/
api.getDrawTime = function() {
return drawTime;
},

/**
* draw all dirty objects/regions
*/
api.draw = function(context) {
if (me.debug.displayDrawTime)
drawTime = Date.now();

This comment has been minimized.

Sign in to view

Copy link
@melonjs

melonjs Sep 11, 2012

Collaborator

I wa wondering : since the debug panel is the last object to be updated and also to be drawn, instead of adding an if statement in the draw method here, do you think we could achieve the same thing by taking the start "date" from the panel update function (since it's in theory the last update function called), and then check the delta once the panel draw function is called . (since it's the last draw function called as well) ?

This comment has been minimized.

Sign in to view

Copy link
@melonjs

melonjs Sep 11, 2012

Collaborator

me.timer.date is better and already contains the same cached information :)

This comment has been minimized.

Sign in to view

Copy link
@parasyte

parasyte Sep 11, 2012

Collaborator

Ooooo! Good point! I like that much better. DebugPanel.update -> DebugPanel.draw should be perfect.

Keep in mind we can't use the cached time because it won't be sufficiently accurate (actually the delta will always be zero if both timestamps use the cached value.
// if feature disable, we only have one dirty rect (the viewport area)
for ( var r = dirtyRects.length, rect; r--, rect = dirtyRects[r];) {
for (var r = dirtyRects.length, rect; r--, rect = dirtyRects[r];) {
// parse all objects
for ( var o = dirtyObjects.length, obj; o--,
obj = dirtyObjects[o];) {
for (var o = dirtyObjects.length, obj; o--, obj = dirtyObjects[o];) {
// if dirty region enabled, make sure the object is in the area to be refreshed
if (me.sys.dirtyRegion && obj.isSprite
&& !obj.overlaps(rect)) {
if (me.sys.dirtyRegion && !obj.getRect().overlaps(rect)) {
continue;
}
// draw the object using the dirty area to be updated
Expand Down Expand Up @@ -1323,16 +1333,27 @@ var me = me || {};

/**
* returns the amount of object being draw per frame<br>
* @name me.game#getEntityCount
* @name me.game#getDrawCount
* @protected
* @function
* @return {Number} the amount of object entities
* @return {Number} the amount of draws
*/
api.getDrawCount = function()
{
return drawManager.getDrawCount();
};

/**
* returns the time when the last frame draw started<br>
* @name me.game#getDrawTime
* @protected
* @function
* @return {Number} the time of frame draw start in milliseconds
*/
api.getDrawTime = function() {
return drawManager.getDrawTime();
},


/**
* return the entity corresponding to the specified GUID<br>
Expand Down Expand Up @@ -1419,7 +1440,7 @@ var me = me || {};
drawManager.makeDirty(obj, updated, updated ? oldRect : null);
}
// update the camera/viewport
if (api.viewport.update(drawManager.isDirty)) {
if (api.viewport.update(drawManager.isDirty) && !me.sys.dirtyRegion) {
drawManager.makeAllDirty();
}
};
Expand Down
13 changes: 13 additions & 0 deletions src/debug/debug.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@
*/
displayFPS : false,

/**
* display average time spent drawing each frame<br>
* default value : false<br>
* @type {Boolean}
* @memberOf me.debug
*/
displayDrawTime : false,

/**
* render object Rectangle & Collision Box<br>
* default value : false
Expand Down Expand Up @@ -72,6 +80,7 @@
// make sure at least the FPS
// counter is enabled
me.debug.displayFPS = true;
me.debug.displayDrawTime = true;
}
};

Expand Down Expand Up @@ -248,6 +257,10 @@
//fps counter
var fps_str = "" + me.timer.fps + "/" + me.sys.fps + " fps";
this.font.draw(context, fps_str, this.width - this.fps_str_len - 5, 5);

//draw time
this.drawtime_str = "draw time : " + (Date.now() - me.game.getDrawTime());
this.font.draw(context, this.drawtime_str, 300, 5);

context.restore();

Expand Down
42 changes: 42 additions & 0 deletions src/level/TMXLayer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@

this.visible = true;
this.opacity = 1.0;

this.rect = new me.Rect(new me.Vector2d(-Infinity, -Infinity), Infinity, Infinity);
},

/**
* get layer rectangle
* @private
* @function
*/
getRect : function() {
return this.rect;
},

/**
Expand Down Expand Up @@ -109,6 +120,16 @@
// default opacity
this.opacity = 1.0;

this.rect = new me.Rect(new me.Vector2d(-Infinity, -Infinity), Infinity, Infinity);
},

/**
* get layer rectangle
* @private
* @function
*/
getRect : function() {
return this.rect;
},

/**
Expand Down Expand Up @@ -231,6 +252,16 @@

this.isCollisionMap = true;

this.rect = new me.Rect(new me.Vector2d(0, 0), this.realwidth, this.realheight);
},

/**
* get layer rectangle
* @private
* @function
*/
getRect : function() {
return this.rect;
},

/**
Expand Down Expand Up @@ -312,7 +343,18 @@

// the default tileset
this.tileset = tilesets?this.tilesets.getTilesetByIndex(0):null;

this.rect = new me.Rect(new me.Vector2d(0, 0), this.realwidth, this.realheight);
},

/**
* get layer rectangle
* @private
* @function
*/
getRect : function() {
return this.rect;
},

/**
* reset function
Expand Down
4 changes: 2 additions & 2 deletions src/level/TMXRenderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@
var start = this.pixelToTileCoords(viewport.x + rect.pos.x,
viewport.y + rect.pos.y).floorSelf();

var end = this.pixelToTileCoords(viewport.x + rect.pos.x + rect.width + this.tilewidth,
viewport.y + rect.pos.y + rect.height + this.tileheight).ceilSelf();
var end = this.pixelToTileCoords(viewport.x + rect.pos.x + rect.width,
viewport.y + rect.pos.y + rect.height).ceilSelf();

This comment has been minimized.

Sign in to view

Copy link
@melonjs

melonjs Sep 11, 2012

Collaborator

Jason,

Are u sure this does not break map rendering for off-limits cases?
I remember I had to add this extra "padding" to fix a few specific maps.
Have a look at the map loader example, and specifically the perpective,

This comment has been minimized.

Sign in to view

Copy link
@parasyte

parasyte Sep 11, 2012

Collaborator

I had a look at the commit that was made to add these two values, but the commit message didn't provide any examples, and I couldn't logically justify it; if there are strange boundary conditions, the application of the boundaries should be in the pixelToTileCoords method, and not in the caller.

I'll definitely try with the map loader example, now that I know what to look at.

When you say 'perspective' do you mean 'isometric'? There's a similar thing for isometric further down TMXRenderer.js that I didn't touch.

This comment has been minimized.

Sign in to view

Copy link
@parasyte

parasyte Sep 12, 2012

Collaborator

Oh, there's a whole example map called "perspective" which simulates perspective rendering with orthographic tiles! ;P

Well, what you're seeing there is a bug in the renderer when the tilewidth/tileheight do not match between the map and the tileset. I actually created a map using this feature which shows the same bug even with the "fix" in place. The ticket is #105. I'm ok to remove this little workaround now that I know what it's for, and that it needs to be redone anyway.

//ensure we are in the valid tile range
end.x = end.x > this.width ? this.width : end.x;
Expand Down
8 changes: 4 additions & 4 deletions src/math/geometry.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@
*/
Object.defineProperty(this, "right", {
get : function() {
return this.pos.x + this.width;
return (this.pos.x + this.width) || Infinity;
},
configurable : true
});
Expand All @@ -319,7 +319,7 @@
*/
Object.defineProperty(this, "bottom", {
get : function() {
return this.pos.y + this.height;
return (this.pos.y + this.height) || Infinity;
},
configurable : true
});
Expand Down Expand Up @@ -401,7 +401,7 @@
if (this.right !== this.pos.x + this.colPos.x + this.width) {
Object.defineProperty(this, "right", {
get : function() {
return this.pos.x + this.colPos.x + this.width;
return (this.pos.x + this.colPos.x + this.width) || Infinity;
},
configurable : true
});
Expand All @@ -425,7 +425,7 @@
if (this.bottom !== this.pos.y + this.colPos.y + this.height) {
Object.defineProperty(this, "bottom", {
get : function() {
return this.pos.y + this.colPos.y + this.height;
return (this.pos.y + this.colPos.y + this.height) || Infinity;
},
configurable : true
});

This comment has been minimized.

Sign in to view

Copy link
@melonjs

melonjs Sep 11, 2012

Collaborator

For performance reason, is it not then better to rather set the default value of width/height to infinity in the class definition or/and the constructor?

This comment has been minimized.

Sign in to view

Copy link
@parasyte

parasyte Sep 11, 2012

Collaborator

Hehe! That was also my original thinking. But this was still needed for infinite planes because -Infinity + Infinity === NaN! We can't have the 'right' or 'bottom' properties returning NaN, as it severely breaks the rectangle geometry methods.

We could still default rectangles to infinite planes, however.

In terms of performance, treating infinite planes as a special case would work ... Just use a different function for the getter that returns Infinity. I'll do that.
Expand Down

0 comments on commit 3b005e7

Please sign in to comment.