Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

Add support for UNSIGNED_BYTE textures #109

Merged
merged 32 commits into from
Oct 9, 2017
Merged

Add support for UNSIGNED_BYTE textures #109

merged 32 commits into from
Oct 9, 2017

Conversation

nsthorat
Copy link
Contributor

@nsthorat nsthorat commented Sep 8, 2017

This enables iOS.

We also add new unit test capability to run tests against multiple environment features.

This change is Reviewable

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@nsthorat nsthorat requested a review from dsmilkov October 8, 2017 22:36
@dsmilkov
Copy link
Contributor

dsmilkov commented Oct 9, 2017

Reviewed 34 of 38 files at r5.
Review status: all files reviewed at latest revision, 6 unresolved discussions.


src/environment.ts, line 34 at r5 (raw file):

writing

change to 'writing and reading'


src/environment.ts, line 123 at r5 (raw file):

  const frameBufferComplete =
      (gl.checkFramebufferStatus(gl.FRAMEBUFFER) === gl.FRAMEBUFFER_COMPLETE);

not in this PR, but would be good to see if this catches safari desktop. you might need to call gl.readPixels to test


src/test_util.ts, line 159 at r5 (raw file):

(testName: string, tests: Tests[], features?: Features)

extract executor signature into a type


src/math/math_test.ts, line 199 at r5 (raw file):

basicLSTMCell

debug mode here and below


src/math/pool_test.ts, line 179 at r5 (raw file):

// math.minPool

math.minPool shows up twice, remove this block


src/math/unaryop_test.ts, line 87 at r5 (raw file):

// math.abs

math.abs repeated.remove block


Comments from Reviewable

@dsmilkov
Copy link
Contributor

dsmilkov commented Oct 9, 2017

:lgtm_strong:


Review status: all files reviewed at latest revision, 6 unresolved discussions.


Comments from Reviewable

@nsthorat
Copy link
Contributor Author

nsthorat commented Oct 9, 2017

Review status: 29 of 34 files reviewed at latest revision, 6 unresolved discussions.


src/environment.ts, line 34 at r5 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

writing

change to 'writing and reading'

Done.


src/environment.ts, line 123 at r5 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

not in this PR, but would be good to see if this catches safari desktop. you might need to call gl.readPixels to test

Added to my todo list.


src/test_util.ts, line 159 at r5 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

(testName: string, tests: Tests[], features?: Features)

extract executor signature into a type

Done.


src/math/math_test.ts, line 199 at r5 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

basicLSTMCell

debug mode here and below

Done.


src/math/pool_test.ts, line 179 at r5 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

// math.minPool

math.minPool shows up twice, remove this block

Done.


src/math/unaryop_test.ts, line 87 at r5 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

// math.abs

math.abs repeated.remove block

Done.


Comments from Reviewable

@nsthorat nsthorat merged commit f164662 into master Oct 9, 2017
@nsthorat nsthorat changed the title iOS Add support for UNSIGNED_BYTE textures Oct 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants