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

[v9] fix!: don't automatically set texture colorSpace, colorMap fallback #3163

Merged
merged 3 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 25 additions & 9 deletions packages/fiber/src/core/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,23 @@ export function diffProps<T = any>(
type ClassConstructor = { new (): void }
const __DEV__ = typeof process !== 'undefined' && process.env.NODE_ENV !== 'production'

// const LinearEncoding = 3000
const sRGBEncoding = 3001
const SRGBColorSpace = 'srgb'
const LinearSRGBColorSpace = 'srgb-linear'

// https://github.com/mrdoob/three.js/pull/27042
// https://github.com/mrdoob/three.js/pull/22748
const colorMaps = [
'map',
'emissiveMap',
'sheenTintMap', // <r134
'sheenColorMap',
'specularTintMap', // <r134
'specularColorMap',
'envMap',
]

// This function applies a set of changes to the instance
export function applyProps<T = any>(object: Instance<T>['object'], props: Instance<T>['props']): Instance<T>['object'] {
const instance = object.__r3f
Expand Down Expand Up @@ -399,10 +416,6 @@ export function applyProps<T = any>(object: Instance<T>['object'], props: Instan
// Alias (output)encoding => (output)colorSpace (since r152)
// https://github.com/pmndrs/react-three-fiber/pull/2829
if (hasColorSpace(root)) {
const sRGBEncoding = 3001
const SRGBColorSpace = 'srgb'
const LinearSRGBColorSpace = 'srgb-linear'

if (key === 'encoding') {
key = 'colorSpace'
value = value === sRGBEncoding ? SRGBColorSpace : LinearSRGBColorSpace
Expand Down Expand Up @@ -441,7 +454,7 @@ export function applyProps<T = any>(object: Instance<T>['object'], props: Instan
// Allow setting array scalars
if (!isColor && target.setScalar && typeof value === 'number') target.setScalar(value)
// Otherwise just set ...
else if (value !== undefined) target.set(value)
else if (value !== undefined) target.set(value) // TODO: unreachable

// For versions of three which don't support THREE.ColorManagement,
// Auto-convert sRGB colors
Expand All @@ -452,18 +465,21 @@ export function applyProps<T = any>(object: Instance<T>['object'], props: Instan
else {
root[key] = value

// Auto-convert sRGB textures, for now ...
// Auto-convert sRGB texture parameters for built-in materials
// https://github.com/pmndrs/react-three-fiber/issues/344
// https://github.com/mrdoob/three.js/pull/25857
if (
rootState &&
!rootState.linear &&
colorMaps.includes(key) &&
root[key] instanceof THREE.Texture &&
// sRGB textures must be RGBA8 since r137 https://github.com/mrdoob/three.js/pull/23129
root[key].format === THREE.RGBAFormat &&
root[key].type === THREE.UnsignedByteType
) {
const texture = root[key] as THREE.Texture
if (hasColorSpace(texture) && hasColorSpace(rootState.gl)) texture.colorSpace = rootState.gl.outputColorSpace
else texture.encoding = rootState.gl.outputEncoding
// NOTE: this cannot be set from the renderer (e.g. sRGB source textures rendered to P3)
if (hasColorSpace(root[key])) root[key].colorSpace = 'srgb'
else root[key].encoding = sRGBEncoding
}
}
}
Expand Down
9 changes: 5 additions & 4 deletions packages/fiber/tests/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ describe('createRoot', () => {
let texture: THREE.Texture & { colorSpace?: string } = null!

let key = 0
function Test({ colorSpace = false }) {
function Test() {
gl = useThree((state) => state.gl)
texture = new THREE.Texture()
return <meshBasicMaterial key={key++} map={texture} />
Expand Down Expand Up @@ -221,19 +221,20 @@ describe('createRoot', () => {
// Sets outputColorSpace since r152
const SRGBColorSpace = 'srgb'
const LinearSRGBColorSpace = 'srgb-linear'
const NoColorSpace = ''

await act(async () =>
createRoot()
.configure({ linear: true })
.render(<Test colorSpace />),
.render(<Test />),
)
expect(gl.outputColorSpace).toBe(LinearSRGBColorSpace)
expect(texture.colorSpace).toBe(LinearSRGBColorSpace)
expect(texture.colorSpace).toBe(NoColorSpace)

await act(async () =>
createRoot()
.configure({ linear: false })
.render(<Test colorSpace />),
.render(<Test />),
)
expect(gl.outputColorSpace).toBe(SRGBColorSpace)
expect(texture.colorSpace).toBe(SRGBColorSpace)
Expand Down