Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browser fixes #479

Merged
merged 42 commits into from
May 18, 2018
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
beee719
Fix safari 0-data buffer setup
dy May 4, 2018
5339bf4
Fix extra logging
dy May 4, 2018
291fab9
Preinit texture with data
dy May 4, 2018
fc8c624
Remove extra comment
dy May 4, 2018
d8b91f5
Fix undefined Dtype
dy May 4, 2018
0b87f34
Add firefox interval timeout
dy May 4, 2018
9a7d9e5
Fix IE context creation
dy May 4, 2018
0703765
Merge branch 'master' into browser-fixes
dy May 14, 2018
16b7628
Use pool to preallocate texture
dy May 14, 2018
34d4b04
Fix test
dy May 14, 2018
50afc01
Add zero-pool for texture data
dy May 15, 2018
462f27c
Remove insignificant checks
dy May 15, 2018
be3326d
Move browser entry to tests
dy May 15, 2018
d18b56a
Move browser entry to test
dy May 15, 2018
1487bd6
Fix losing extension on context loss
dy May 15, 2018
127e0a0
Rename half_float to half_float_oes
dy May 15, 2018
85bef7d
Init zero-data texture on resize
dy May 15, 2018
23ba463
Ignore null allocation
dy May 15, 2018
b6f8d25
Revert test/index
dy May 15, 2018
9938a8e
Fix unsupported premultipliedAlpha in IE
dy May 15, 2018
54db288
Report a safari-specific test failure without halting all tests
May 14, 2018
202e395
Fix formatting
May 14, 2018
c6abb2e
1.3.2
dy May 16, 2018
0aa1b01
Fix links
dy May 16, 2018
d369dbf
Merge branch 'master' into browser-fixes
dy May 16, 2018
ac33e4f
Remove msg from code
dy May 16, 2018
449b5b5
Fix buffer test
dy May 16, 2018
1da66e1
Remove half float allocation - that breaks in safari
dy May 16, 2018
fc418cc
Unroll tests
dy May 16, 2018
0a82ba4
Fix safari framebuffer test
dy May 17, 2018
6afda7c
Display warning reading floats in contexts not supporting the feature
dy May 17, 2018
ad1c6ac
Rename mikolalysenko with regl-project
dy May 17, 2018
1a33343
Fix IE shader issues
dy May 17, 2018
3199462
Fix IE none colorspace test
dy May 17, 2018
617ff84
Add pow2 cube texture check
dy May 17, 2018
87e2ebf
Fix cube FBO test
dy May 17, 2018
b458147
Fix rest of IE tests
dy May 17, 2018
8addf24
Clean up
dy May 17, 2018
7745514
Move feature-detection to limits
dy May 18, 2018
0b31a64
Add API lines
dy May 18, 2018
71bba6c
Rename cubeNpot to npotCubeTexture
dy May 18, 2018
67d4cdd
Rename npotCubeTexture to npotTextureCube
dy May 18, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ module.exports = function wrapBufferState (gl, stats, config, attributeState) {

buffer.bind()
if (!data) {
gl.bufferData(buffer.type, byteLength, usage)
// #475
if (byteLength) gl.bufferData(buffer.type, byteLength, usage)
buffer.dtype = dtype || GL_UNSIGNED_BYTE
buffer.usage = usage
buffer.dimension = dimension
Expand Down
1 change: 0 additions & 1 deletion lib/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -3278,7 +3278,6 @@ module.exports = function reglCore (
emitPollFramebuffer(env, refresh, null, true)

// Refresh updates all attribute state changes
var extInstancing = gl.getExtension('angle_instanced_arrays')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This enables ANGLE_instanced_arrays for every regl call, which makes Firefox display warnings on loseContext/restoreContext

var INSTANCING
if (extInstancing) {
INSTANCING = env.link(extInstancing)
Expand Down
31 changes: 31 additions & 0 deletions lib/read.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ module.exports = function wrapReadPixels (
context,
glAttributes,
extensions) {
var supportsFloat = extensions.oes_texture_float && isFloatSupported(gl)

function readPixelsImpl (input) {
var type
if (framebufferState.next === null) {
Expand All @@ -30,6 +32,10 @@ module.exports = function wrapReadPixels (
check(
type === GL_UNSIGNED_BYTE || type === GL_FLOAT,
'Reading from a framebuffer is only allowed for the types \'uint8\' and \'float\'')

if (type === GL_FLOAT) {
check(supportsFloat, 'Reading \'float\' values is not permitted in your browser. For a fallback, please see: https://www.npmjs.com/package/glsl-read-float')
}
} else {
check(
type === GL_UNSIGNED_BYTE,
Expand Down Expand Up @@ -128,3 +134,28 @@ module.exports = function wrapReadPixels (

return readPixels
}

function isFloatSupported (gl) {
var texture = gl.createTexture()
gl.bindTexture(gl.TEXTURE_2D, texture)
gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, 1, 1, 0, gl.RGBA, gl.FLOAT, null)

var fbo = gl.createFramebuffer()
gl.bindFramebuffer(gl.FRAMEBUFFER, fbo)
gl.framebufferTexture2D(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.TEXTURE_2D, texture, 0)
gl.bindTexture(gl.TEXTURE_2D, null)
if (gl.checkFramebufferStatus(gl.FRAMEBUFFER) !== gl.FRAMEBUFFER_COMPLETE) return false

gl.viewport(0, 0, 1, 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should not affect previous render/framebuffer, should that? At the moment of this call gl does not have any fbo/renderbuffer attached.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and apologies, but just curious? What does this really do? On platforms where it's not supported, does it currently fail silently? Throw an error? A warning? After this, what is the result?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This uses own regl ‘check’ function to throw an error if reading floats is not supported, as discussed in the adjacent PR.

Copy link
Member

@rreusser rreusser May 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, but I forgot: does it already throw an error without regl itself detecting and throwing an error?

Copy link
Member

@rreusser rreusser May 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#487 (comment)

instead of silent failure

never mind. got it 👍

gl.clearColor(1.0, 0.0, 0.0, 1.0)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if that can lose previous clear color, if there is any. That should not be an issue, since regl.clear or regl._refresh fully reinit the state.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is run on startup, right? I think it'd be fine then as long as the state is refreshed/initialized afterwards.

gl.clear(gl.COLOR_BUFFER_BIT)
var pixels = new Float32Array(4)
gl.readPixels(0, 0, 1, 1, gl.RGBA, gl.FLOAT, pixels)

if (gl.getError()) return false

gl.deleteFramebuffer(fbo)
gl.deleteTexture(texture)

return pixels[0] === 1.0
}
29 changes: 24 additions & 5 deletions lib/texture.js
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,7 @@ module.exports = function createTextureSet (
var type = info.type
var width = info.width
var height = info.height
var channels = info.channels

setFlags(info)

Expand All @@ -839,8 +840,16 @@ module.exports = function createTextureSet (
gl.copyTexImage2D(
target, miplevel, format, info.xOffset, info.yOffset, width, height, 0)
} else {
gl.texImage2D(
target, miplevel, format, width, height, 0, format, type, data)
var nullData = !data
if (nullData) {
data = pool.zero.allocType(type, width * height * channels)
}

gl.texImage2D(target, miplevel, format, width, height, 0, format, type, data)

if (nullData && data) {
pool.zero.freeType(data)
}
}
}

Expand Down Expand Up @@ -1324,17 +1333,27 @@ module.exports = function createTextureSet (
reglTexture2D.height = texture.height = h

tempBind(texture)

var data
var channels = texture.channels
var type = texture.type

for (var i = 0; texture.mipmask >> i; ++i) {
var _w = w >> i
var _h = h >> i
if (!_w || !_h) break
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firefox warns when w/h is not POT, and it is not when _w === 0

data = pool.zero.allocType(type, _w * _h * channels)
gl.texImage2D(
GL_TEXTURE_2D,
i,
texture.format,
w >> i,
h >> i,
_w,
_h,
0,
texture.format,
texture.type,
null)
data)
if (data) pool.zero.freeType(data)
}
tempRestore()

Expand Down
111 changes: 59 additions & 52 deletions lib/util/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ var GL_INT = 5124
var GL_UNSIGNED_INT = 5125
var GL_FLOAT = 5126

var bufferPool = loop(8, function () {
return []
})

function nextPow16 (v) {
for (var i = 16; i <= (1 << 28); i *= 16) {
if (v <= i) {
Expand All @@ -34,59 +30,70 @@ function log2 (v) {
return r | (v >> 1)
}

function alloc (n) {
var sz = nextPow16(n)
var bin = bufferPool[log2(sz) >> 2]
if (bin.length > 0) {
return bin.pop()
function createPool () {
var bufferPool = loop(8, function () {
return []
})

function alloc (n) {
var sz = nextPow16(n)
var bin = bufferPool[log2(sz) >> 2]
if (bin.length > 0) {
return bin.pop()
}
return new ArrayBuffer(sz)
}
return new ArrayBuffer(sz)
}

function free (buf) {
bufferPool[log2(buf.byteLength) >> 2].push(buf)
}
function free (buf) {
bufferPool[log2(buf.byteLength) >> 2].push(buf)
}

function allocType (type, n) {
var result = null
switch (type) {
case GL_BYTE:
result = new Int8Array(alloc(n), 0, n)
break
case GL_UNSIGNED_BYTE:
result = new Uint8Array(alloc(n), 0, n)
break
case GL_SHORT:
result = new Int16Array(alloc(2 * n), 0, n)
break
case GL_UNSIGNED_SHORT:
result = new Uint16Array(alloc(2 * n), 0, n)
break
case GL_INT:
result = new Int32Array(alloc(4 * n), 0, n)
break
case GL_UNSIGNED_INT:
result = new Uint32Array(alloc(4 * n), 0, n)
break
case GL_FLOAT:
result = new Float32Array(alloc(4 * n), 0, n)
break
default:
return null
function allocType (type, n) {
var result = null
switch (type) {
case GL_BYTE:
result = new Int8Array(alloc(n), 0, n)
break
case GL_UNSIGNED_BYTE:
result = new Uint8Array(alloc(n), 0, n)
break
case GL_SHORT:
result = new Int16Array(alloc(2 * n), 0, n)
break
case GL_UNSIGNED_SHORT:
result = new Uint16Array(alloc(2 * n), 0, n)
break
case GL_INT:
result = new Int32Array(alloc(4 * n), 0, n)
break
case GL_UNSIGNED_INT:
result = new Uint32Array(alloc(4 * n), 0, n)
break
case GL_FLOAT:
result = new Float32Array(alloc(4 * n), 0, n)
break
default:
return null
}
if (result.length !== n) {
return result.subarray(0, n)
}
return result
}
if (result.length !== n) {
return result.subarray(0, n)

function freeType (array) {
free(array.buffer)
}
return result
}

function freeType (array) {
free(array.buffer)
return {
alloc: alloc,
free: free,
allocType: allocType,
freeType: freeType
}
}

module.exports = {
alloc: alloc,
free: free,
allocType: allocType,
freeType: freeType
}
module.exports = createPool()

// zero pool for initial zero data
module.exports.zero = createPool()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reserved for zero-data inits.

4 changes: 2 additions & 2 deletions lib/webgl.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ function createCanvas (element, onDone, pixelRatio) {
}
}

function createContext (canvas, contexAttributes) {
function createContext (canvas, contextAttributes) {
function get (name) {
try {
return canvas.getContext(name, contexAttributes)
return canvas.getContext(name, contextAttributes)
} catch (e) {
return null
}
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
"google-closure-compiler": "^20160315.2.0",
"hsv2rgb": "^1.1.0",
"install": "^0.8.1",
"is-iexplorer": "^1.0.0",
"is-safari": "^1.0.0",
"is-travis": "^2.0.0",
"istanbul": "^0.4.3",
"mkdirp": "^0.5.1",
Expand Down Expand Up @@ -92,7 +94,7 @@
},
"scripts": {
"test": "standard | snazzy && tape test/index.js | tap-min",
"test-browser": "budo test/util/browser.js --open",
"test-browser": "budo test/browser.js --open",
"test-typescript": "tsc",
"cover": "istanbul cover test/index.js",
"bench-browser": "budo bench/bench.js --open",
Expand Down
4 changes: 2 additions & 2 deletions test/attributes-nested.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ tape('attributes nested', function (t) {

function checkPixels (expected) {
var actual = regl.read()
console.log('actual: ', actual)
console.log('expected: ', expected)
// console.log('actual: ', actual)
// console.log('expected: ', expected)
for (var i = 0; i < 5 * 5; ++i) {
if (!!actual[4 * i] !== !!expected[i]) {
console.log('fail at: ', i)
Expand Down
2 changes: 1 addition & 1 deletion test/util/browser.js → test/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,4 @@ function updateDOM () {
pendingRaf = null
}

require('../index')
require('./index')
10 changes: 4 additions & 6 deletions test/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,12 @@ tape('buffer arg parsing', function (t) {
t.same(bufferProps[prop], props[prop], prefix + '.' + prop)
})
gl.bindBuffer(gl.ARRAY_BUFFER, bufferProps.buffer)
gl.getBufferParameter(
t.same(gl.getBufferParameter(
gl.ARRAY_BUFFER,
gl.BUFFER_SIZE,
bufferProps.byteLength)
gl.getBufferParameter(
gl.BUFFER_SIZE), bufferProps.byteLength, prefix + ' gl.buffer size')
t.same(gl.getBufferParameter(
gl.ARRAY_BUFFER,
gl.BUFFER_USAGE,
bufferProps.usage)
gl.BUFFER_USAGE), bufferProps.usage, prefix + ' gl.buffer usage')
}

checkProperties(
Expand Down
3 changes: 2 additions & 1 deletion test/constructor.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
var tape = require('tape')
var createContext = require('./util/create-context')
var createREGL = require('../regl')
var ie = require('is-iexplorer')

tape('regl constructor', function (t) {
// loading from a bad selector string breaks
Expand Down Expand Up @@ -140,7 +141,7 @@ tape('regl constructor', function (t) {
}
})
t.equals(regl._gl.canvas, canvas, 'create from canvas ok')
t.equals(regl.attributes.premultipliedAlpha, false, 'create from attributes ok')
!ie && t.equals(regl.attributes.premultipliedAlpha, false, 'create from attributes ok')
regl.destroy()

var container = document.createElement('div')
Expand Down
1 change: 1 addition & 0 deletions test/context-loss.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ function testContextLoss (t, gl, extLoseContext, onDone) {

function pollTests () {
if (tests.length === 0) {
t.equals(gl.getError(), 0, 'error ok')
regl.destroy()
onDone()
return
Expand Down
3 changes: 1 addition & 2 deletions test/elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,12 @@ tape('elements', function (t) {
t.ok('destroy successful')
}
]

var poll = setInterval(function () {
if (cases.length === 0) {
clearInterval(poll)
t.end()
} else {
(cases.shift())()
}
})
}, 0)
})
Loading