R151 redux by elalish · Pull Request #4192 · google/model-viewer (original) (raw)

Skip to content

Navigation Menu

Sign in

Appearance settings

• • AI CODE CREATION
    * GitHub CopilotWrite better code with AI
    * GitHub SparkBuild and deploy intelligent apps
    * GitHub ModelsManage and compare prompts
    * MCP RegistryNewIntegrate external tools
  • DEVELOPER WORKFLOWS
    * ActionsAutomate any workflow
    * CodespacesInstant dev environments
    * IssuesPlan and track work
    * Code ReviewManage code changes
  • APPLICATION SECURITY
    * GitHub Advanced SecurityFind and fix vulnerabilities
    * Code securitySecure your code as you build
    * Secret protectionStop leaks before they start
  • EXPLORE
    * Why GitHub
    * Documentation
    * Blog
    * Changelog
    * Marketplace

View all features

• • BY COMPANY SIZE
    * Enterprises
    * Small and medium teams
    * Startups
    * Nonprofits
  • BY USE CASE
    * App Modernization
    * DevSecOps
    * DevOps
    * CI/CD
    * View all use cases
  • BY INDUSTRY
    * Healthcare
    * Financial services
    * Manufacturing
    * Government
    * View all industries

View all solutions

• • EXPLORE BY TOPIC
    * AI
    * Software Development
    * DevOps
    * Security
    * View all topics
  • EXPLORE BY TYPE
    * Customer stories
    * Events & webinars
    * Ebooks & reports
    * Business insights
    * GitHub Skills
  • SUPPORT & SERVICES
    * Documentation
    * Customer support
    * Community forum
    * Trust center
    * Partners

View all resources

• • COMMUNITY
    * GitHub SponsorsFund open source developers
  • PROGRAMS
    * Security Lab
    * Maintainer Community
    * Accelerator
    * GitHub Stars
    * Archive Program
  • REPOSITORIES
    * Topics
    * Trending
    * Collections
• • ENTERPRISE SOLUTIONS
    * Enterprise platformAI-powered developer platform
  • AVAILABLE ADD-ONS
    * GitHub Advanced SecurityEnterprise-grade security features
    * Copilot for BusinessEnterprise-grade AI features
    * Premium SupportEnterprise-grade 24/7 support
• Pricing

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Sign in

Sign up

Appearance settings

google / model-viewer Public

• Notifications You must be signed in to change notification settings

• Fork926

• Star 8.1k

• Code

• Issues 103

• Pull requests 9

• Discussions

• Actions

• Projects

• Wiki

• Security and quality

• Insights

Additional navigation options

Merged

elalish

merged 6 commits into

masterfrom

r151

Apr 1, 2023

ConversationCommits (6)ChecksFiles changed

Merged

R151 redux#4192

elalish

merged 6 commits into

masterfrom

r151

Conversation

Copy link Copy Markdown

Contributor

elalish commented

Mar 31, 2023

Re-landing #4186

Apparently we have to specify that legacy lighting is a bad idea in PBR...

elalish added 5 commits

March 30, 2023 16:40

[updated to three.js r151](/google/model-viewer/pull/4192/commits/dd30acc288c1ace7307fef1c3652c82410777b82 "updated to three.js r151")

[dd30acc](/google/model-viewer/pull/4192/commits/dd30acc288c1ace7307fef1c3652c82410777b82)

[r151.2](/google/model-viewer/pull/4192/commits/79c5642bb281deef1324735cd0200a62865dbbe9 "r151.2")

[79c5642](/google/model-viewer/pull/4192/commits/79c5642bb281deef1324735cd0200a62865dbbe9)

[updated goldens](/google/model-viewer/pull/4192/commits/7e42e74608a7369d4a0fda3a7954622ef8f023e8 "updated goldens")

[7e42e74](/google/model-viewer/pull/4192/commits/7e42e74608a7369d4a0fda3a7954622ef8f023e8)

[Merge branch 'master' of github.com:google/model-viewer into r151](/google/model-viewer/pull/4192/commits/0fb7485b9116bd379f1ddfaae690b7fa105179d5 "Merge branch 'master' of github.com:google/model-viewer into r151")

[0fb7485](/google/model-viewer/pull/4192/commits/0fb7485b9116bd379f1ddfaae690b7fa105179d5)

[DO NOT USE LEGACY LIGHTS](/google/model-viewer/pull/4192/commits/5b45815026490966f3a95d1dce973b2b69d8d38c "DO NOT USE LEGACY LIGHTS")

[5b45815](/google/model-viewer/pull/4192/commits/5b45815026490966f3a95d1dce973b2b69d8d38c)

elalish self-assigned this

Mar 31, 2023

[fix two goldens](/google/model-viewer/pull/4192/commits/01fe1b484911427790f29c320cfeb9a395e91207 "fix two goldens")

[01fe1b4](/google/model-viewer/pull/4192/commits/01fe1b484911427790f29c320cfeb9a395e91207)

elalish mentioned this pull request

Apr 1, 2023

Revert "updated to three.js r151 (#4186)"#4191

Merged

elalish merged commit 2846fee into master

Apr 1, 2023

elalish deleted the r151 branch

April 1, 2023 00:32

Copy link Copy Markdown

Collaborator

mrdoob commented

Apr 3, 2023

Hmm... didn't this piece of code warn you?

https://github.com/mrdoob/three.js/blob/8400dc53eff51b434ae965edefa387e7695f9e42/src/renderers/WebGLRenderer.js#L2296-L2308

Or was the fact the fact that the boolean got inverted that confused you?

/cc @donmccurdy

Copy link Copy Markdown

Collaborator

mrdoob commented

Apr 3, 2023

Oh... Maybe because of the types?

mrdoob/three.js#25744

Copy link Copy Markdown

Contributor

donmccurdy commented

Apr 3, 2023

It'd be preferable if the type definitions did something like this, though I imagine it's more trouble to maintain:

/**

• @deprecated Replaced by .useLegacyLights in r151+
• @default false */ physicallyCorrectLights: boolean;

/** @default true */ useLegacyLights: boolean;

Copy link Copy Markdown

Contributor Author

elalish commented

Apr 3, 2023

Yeah, I was just going too fast. I had assumed anything called "useLegacy" would default to false.

Copy link Copy Markdown

Contributor

donmccurdy commented

Apr 3, 2023

One day... 😢

JL-Vidinoti pushed a commit to vidinoti/model-viewer that referenced this pull request

Apr 22, 2024

`[R151 redux (](/vidinoti/model-viewer/commit/4201f7db9cb1ca599eaf8b9b6e0ae9d0c4e4cd88 "R151 redux (#4192)

• updated to three.js r151

• r151.2

• updated goldens

• DO NOT USE LEGACY LIGHTS

• fix two goldens")google#4192[)](/vidinoti/model-viewer/commit/4201f7db9cb1ca599eaf8b9b6e0ae9d0c4e4cd88 "R151 redux (#4192)

• updated to three.js r151

• r151.2

• updated goldens

• DO NOT USE LEGACY LIGHTS

• fix two goldens") `

[4201f7d](/vidinoti/model-viewer/commit/4201f7db9cb1ca599eaf8b9b6e0ae9d0c4e4cd88)

• updated to three.js r151

• r151.2

• updated goldens

• DO NOT USE LEGACY LIGHTS

• fix two goldens

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Sign up for free to join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

elalish

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants