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

Revert "Tonemapping example refactor " #2

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

DGriffin91
Copy link
Owner

Reverts #1

@github-actions
Copy link

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@DGriffin91 DGriffin91 merged commit 291ce1f into tonemap_options Feb 16, 2023
DGriffin91 pushed a commit that referenced this pull request Aug 26, 2023
# Objective

> ℹ️ **This is an adoption of bevyengine#4081 by @james7132**

Fixes bevyengine#4080.

Provide a way to pre-parse reflection paths so as to avoid having to parse at each call to `GetPath::path` (or similar method).

## Solution

Adds the `ParsedPath` struct (named `FieldPath` in the original PR) that parses and caches the sequence of accesses to a reflected element. This is functionally similar to the `GetPath` trait, but removes the need to parse an unchanged path more than once.

### Additional Changes

Included in this PR from the original is cleaner code as well as the introduction of a new pathing operation: field access by index. This allows struct and struct variant fields to be accessed in a more performant (albeit more fragile) way if needed. This operation is faster due to not having to perform string matching. As an example, if we wanted the third field on a struct, we'd write `#2`—where `#` denotes indexed access and `2` denotes the desired field index.

This PR also contains improved documentation for `GetPath` and friends, including renaming some of the methods to be more clear to the end-user with a reduced risk of getting them mixed up.

### Future Work

There are a few things that could be done as a separate PR (order doesn't matter— they could be followup PRs or done in parallel). These are:

- [x] ~~Add support for `Tuple`. Currently, we hint that they work but they do not.~~ See bevyengine#7324
- [ ] Cleanup `ReflectPathError`. I think it would be nicer to give `ReflectPathError` two variants: `ReflectPathError::ParseError` and `ReflectPathError::AccessError`, with all current variants placed within one of those two. It's not obvious when one might expect to receive one type of error over the other, so we can help by explicitly categorizing them.

---

## Changelog

- Cleaned up `GetPath` logic
- Added `ParsedPath` for cached reflection paths
- Added new reflection path syntax: struct field access by index (example syntax: `foo#1`)
- Renamed methods on `GetPath`:
  - `path` -> `reflect_path`
  - `path_mut` -> `reflect_path_mut`
  - `get_path` -> `path`
  - `get_path_mut` -> `path_mut`

## Migration Guide

`GetPath` methods have been renamed according to the following:
- `path` -> `reflect_path`
- `path_mut` -> `reflect_path_mut`
- `get_path` -> `path`
- `get_path_mut` -> `path_mut`


Co-authored-by: Gino Valente <[email protected]>
DGriffin91 pushed a commit that referenced this pull request Oct 17, 2024
…3906)

# Objective

- Second part of bevyengine#13900 
- based on bevyengine#13905 

## Solution

- check_dir_light_mesh_visibility defers setting the entity's
`ViewVisibility `so that Bevy can schedule it to run in parallel with
`check_point_light_mesh_visibility`.

- Reduce HashMap lookups for directional light checking as much as
possible

- Use `par_iter `to parallelize the checking process within each system.

---------

Co-authored-by: Kristoffer Søholm <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant