Skip to content

dont normalize twice for no reason in octahedral_decode#20190

Merged
mockersf merged 1 commit intobevyengine:mainfrom
atlv24:ad/oct-dec-fix
Jul 19, 2025
Merged

dont normalize twice for no reason in octahedral_decode#20190
mockersf merged 1 commit intobevyengine:mainfrom
atlv24:ad/oct-dec-fix

Conversation

@atlv24
Copy link
Contributor

@atlv24 atlv24 commented Jul 18, 2025

Objective

  • dont normalize twice for no reason

Solution

  • dont normalize twice for no reason

Testing

  • trivial

@JMS55 JMS55 added D-Trivial Nice and easy! A great choice to get started with Bevy A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change labels Jul 18, 2025
@JMS55 JMS55 requested a review from alice-i-cecile July 18, 2025 16:35
@atlv24 atlv24 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 18, 2025
Jondolf
Jondolf approved these changes Jul 18, 2025
View reviewed changes
Copy link
Contributor

@Jondolf Jondolf left a comment

Choose a reason for hiding this comment

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

dont normalize twice for no reason

@hymm
Copy link
Contributor

hymm commented Jul 18, 2025

pr title should probably mention octahedral_decode as it'll be a bit vague to see in the commit history otherwise.

@atlv24 atlv24 changed the title dont normalize twice for no reason dont normalize twice for no reason in octahedral_decode Jul 18, 2025
@atlv24
Copy link
Contributor Author

atlv24 commented Jul 18, 2025

I think we should block merging this on getting more approvals.

@janhohenheim janhohenheim added the X-Uncontroversial This work is generally agreed upon label Jul 18, 2025
@janhohenheim
Copy link
Member

janhohenheim commented Jul 18, 2025

I think it's fair to say that this is on the X-Uncontroversial side

tautropfli
tautropfli approved these changes Jul 18, 2025
View reviewed changes
Copy link
Contributor

@tautropfli tautropfli left a comment

Choose a reason for hiding this comment

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

Lgtm

@janhohenheim janhohenheim added this to the 0.17 milestone Jul 18, 2025
@janhohenheim janhohenheim added the C-Performance A change motivated by improving speed, memory usage or compile times label Jul 18, 2025
@NthTensor
Copy link
Contributor

NthTensor commented Jul 18, 2025

... Will this be the first bevy PR to be officially reviewed by copilot?

CorvusPrudens
CorvusPrudens approved these changes Jul 18, 2025
View reviewed changes
Copy link
Member

@CorvusPrudens CorvusPrudens left a comment

Choose a reason for hiding this comment

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

Sometimes it's good to be a little less normal.

@k2d222
Copy link
Contributor

k2d222 commented Jul 18, 2025

Since everyone is paying attention- could you guys stop leaving your dirty cups in the sink? it takes only two minutes to empty the dishwasher!! While I'm at it: cuttlery is better stored with the handle pointing up in the baskets. I don't want to scratch my arms on your pointy filthy knives. Also I know it's an unpopular opinion, but storing the cups upside down doesn't let them dry up completely, and it leaves a ring of humidity that's not too pleasant IMO.

Thank you for keeping our shared spaces tidy and keep up the good work!

LikeLakers2
LikeLakers2 approved these changes Jul 19, 2025
View reviewed changes
Copy link
Contributor

@LikeLakers2 LikeLakers2 left a comment

Choose a reason for hiding this comment

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

Officially approved by a trans person (me).

@mockersf mockersf added this pull request to the merge queue Jul 19, 2025
Merged via the queue into bevyengine:main with commit eabde3d Jul 19, 2025
55 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in Rendering (Old) Jul 19, 2025
@atlv24 atlv24 deleted the ad/oct-dec-fix branch July 23, 2025 23:46
@alice-i-cecile alice-i-cecile mentioned this pull request Aug 20, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@superdump superdump superdump approved these changes

+66 more reviewers

@urben1680 urben1680 urben1680 requested changes

@Pnoenix Pnoenix Pnoenix approved these changes

@djkato djkato djkato approved these changes

@Hexorg Hexorg Hexorg approved these changes

@geckoxx geckoxx geckoxx approved these changes

@LikeLakers2 LikeLakers2 LikeLakers2 approved these changes

@purplg purplg purplg approved these changes

@ramirezmike ramirezmike ramirezmike approved these changes

@benpoulson benpoulson benpoulson approved these changes

@NiseVoid NiseVoid NiseVoid approved these changes

@tautropfli tautropfli tautropfli approved these changes

@Aceeri Aceeri Aceeri approved these changes

@Runi-c Runi-c Runi-c approved these changes

@sneakymizu sneakymizu sneakymizu approved these changes

@nicopap nicopap nicopap approved these changes

@noprobelm noprobelm noprobelm approved these changes

@jeremystucki jeremystucki jeremystucki approved these changes

@sixfold-origami sixfold-origami sixfold-origami approved these changes

@IceSentry IceSentry IceSentry approved these changes

@andresovela andresovela andresovela approved these changes

@ItsDoot ItsDoot ItsDoot approved these changes

@janhohenheim janhohenheim janhohenheim approved these changes

@Mafii Mafii Mafii approved these changes

@iczero iczero iczero approved these changes

@benfrankel benfrankel benfrankel approved these changes

@dmyyy dmyyy dmyyy approved these changes

@TimJentzsch TimJentzsch TimJentzsch approved these changes

@mate-h mate-h mate-h approved these changes

@NthTensor NthTensor NthTensor approved these changes

@meepleek meepleek meepleek approved these changes

@Azorlogh Azorlogh Azorlogh approved these changes

@IQuick143 IQuick143 IQuick143 approved these changes

@Ixentus Ixentus Ixentus approved these changes

@StefanieJaeger StefanieJaeger StefanieJaeger approved these changes

@Zeophlite Zeophlite Zeophlite approved these changes

@maximemulder maximemulder maximemulder approved these changes

@stargazing-dino stargazing-dino stargazing-dino approved these changes

@tim-blackbird tim-blackbird tim-blackbird approved these changes

@CorvusPrudens CorvusPrudens CorvusPrudens approved these changes

@SpecificProtagonist SpecificProtagonist SpecificProtagonist approved these changes

@TrialDragon TrialDragon TrialDragon approved these changes

@dominiksalwerowicz dominiksalwerowicz dominiksalwerowicz approved these changes

@DGriffin91 DGriffin91 DGriffin91 approved these changes

@BenjaminBrienen BenjaminBrienen BenjaminBrienen approved these changes

@PROMETHIA-27 PROMETHIA-27 PROMETHIA-27 approved these changes

@DaAlbrecht DaAlbrecht DaAlbrecht approved these changes

@JMS55 JMS55 JMS55 approved these changes

@Dimev Dimev Dimev approved these changes

@Insprill Insprill Insprill approved these changes

@ecoskey ecoskey ecoskey approved these changes

@Jondolf Jondolf Jondolf approved these changes

@copygirl copygirl copygirl approved these changes

@2406mmartin 2406mmartin 2406mmartin approved these changes

@SoysCodingCafe SoysCodingCafe SoysCodingCafe approved these changes

@stevehello166 stevehello166 stevehello166 approved these changes

@PhantomMorrigan PhantomMorrigan PhantomMorrigan approved these changes

@LukK-Dev LukK-Dev LukK-Dev approved these changes

@tomara-x tomara-x tomara-x approved these changes

@Pleasegivemeamango Pleasegivemeamango Pleasegivemeamango approved these changes

@Peepo-Juice Peepo-Juice Peepo-Juice approved these changes

@sinnom sinnom sinnom approved these changes

@zeozeozeo zeozeozeo zeozeozeo approved these changes

@hamz0640 hamz0640 hamz0640 approved these changes

@Elabajaba Elabajaba Elabajaba approved these changes

@JaySpruce JaySpruce JaySpruce approved these changes

@Zuzka-Cat Zuzka-Cat Zuzka-Cat approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alice-i-cecile alice-i-cecile

Labels

A-Math Fundamental domain-agnostic mathematical operations A-Rendering Drawing game state to the screen A-Utils Utility functions and types C-Code-Quality A section of code that is hard to understand or change C-Performance A change motivated by improving speed, memory usage or compile times D-Shaders This code uses GPU shader languages D-Trivial Nice and easy! A great choice to get started with Bevy P-Critical This must be fixed immediately or contributors or users will be severely impacted S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers X-Uncontroversial This work is generally agreed upon

Projects

No open projects
Rendering (Old)
Status: Done

Milestone

0.17

Development

Successfully merging this pull request may close these issues.