Skip to content

Commit

Permalink
More changes based on review.
Browse files Browse the repository at this point in the history
  • Loading branch information
bagnell committed Feb 28, 2013
1 parent 8ee48e7 commit 8d2937a
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 5 deletions.
2 changes: 1 addition & 1 deletion Source/Scene/PerformanceDisplay.js
Expand Up @@ -134,7 +134,7 @@ define([
}

if (typeof this._quad === 'undefined') {
this._quad = new ViewportQuad(Material.fromType(context, Material.ImageType));
this._quad = new ViewportQuad(undefined, Material.fromType(context, Material.ImageType));
}

if (typeof this._texture === 'undefined') {
Expand Down
12 changes: 8 additions & 4 deletions Source/Scene/ViewportQuad.js
Expand Up @@ -41,14 +41,14 @@ define([
* @alias ViewportQuad
* @constructor
*
* @param {BoundingRectangle} [rectangle] The {@link BoundingRectangle} defining the quad's position within the viewport.
* @param {Material} [material] The {@link Material} defining the surface appearance of the viewport quad.
*
* @example
* var viewportQuad = new ViewportQuad();
* var viewportQuad = new BoundingRectangle(0, 0, 80, 40);
* var viewportQuad = new ViewportQuad(new BoundingRectangle(0, 0, 80, 40));
* viewportQuad.material.uniforms.color = new Color(1.0, 0.0, 0.0, 1.0);
*/
var ViewportQuad = function(material) {
var ViewportQuad = function(rectangle, material) {

this._va = undefined;
this._overlayCommand = new DrawCommand();
Expand All @@ -66,6 +66,10 @@ define([
*/
this.show = true;

if (typeof rectangle === 'undefined') {
rectangle = new BoundingRectangle(0, 0, 10, 10);

This comment has been minimized.

Copy link
@pjcozzi

pjcozzi Feb 28, 2013

Contributor

I don't know the rational behind this default. If we had access to the context, I would make it full-screen. Since we don't, we should probably default construct the BoundingRectangle.

}

/**
* The BoundingRectangle defining the quad's position within the viewport.
*
Expand All @@ -74,7 +78,7 @@ define([
* @example
* viewportQuad.rectangle = new BoundingRectangle(0, 0, 80, 40);
*/
this.rectangle = new BoundingRectangle(0, 0, 10, 10);
this.rectangle = rectangle;

This comment has been minimized.

Copy link
@pjcozzi

pjcozzi Feb 28, 2013

Contributor

BoundingRectangle.clone is probably what the user expects here.


if (typeof material === 'undefined') {
material = Material.fromType(undefined, Material.ColorType);
Expand Down
12 changes: 12 additions & 0 deletions Specs/Scene/ViewportQuadSpec.js
Expand Up @@ -57,6 +57,18 @@ defineSuite([
us = undefined;
});

it('constructs with a rectangle', function() {
var rectangle = new BoundingRectangle(1.0, 2.0, 3.0, 4.0);
var quad = new ViewportQuad(rectangle);
expect(quad.rectangle).toEqual(rectangle);
});

it('constructs with a material', function() {
var material = Material.fromType(undefined, Material.ErosionType);
var quad = new ViewportQuad(undefined, material);
expect(quad.material.type).toEqual(material.type);
});

it('gets the default color', function() {
expect(viewportQuad.material.uniforms.color).toEqual(
new Color(1.0, 1.0, 1.0, 1.0));
Expand Down

0 comments on commit 8d2937a

Please sign in to comment.