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

feat: added 16bit config flags #3292

Closed

Conversation

Ouwen
Copy link
Contributor

@Ouwen Ouwen commented Mar 30, 2023

Context

This adds flags to enable 16 bit texture rendering with preferSizeOverAccuracy always being true if 16bit rendering is enabled.

Testing

export APP_CONFIG=config/default_16bit.js
yarn run dev
# open the viewer to any image and look at the memory profile which should use 16 int or int arraybuffers

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • "Browser: Version 111.0.5563.110 (Official Build) (arm64)

@netlify
Copy link

netlify bot commented Mar 30, 2023

Deploy Preview for ohif-platform-viewer ready!

Name Link
🔨 Latest commit 5377ad4
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-viewer/deploys/642556d5bdc7410008131472
😎 Deploy Preview https://deploy-preview-3292--ohif-platform-viewer.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 30, 2023

Deploy Preview for ohif-platform-docs canceled.

Name Link
🔨 Latest commit 5377ad4
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/642556d5b02f30000850811a

@codecov
Copy link

codecov bot commented Mar 30, 2023

Codecov Report

Merging #3292 (5377ad4) into v3-stable (9f1e813) will increase coverage by 12.64%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##           v3-stable    #3292       +/-   ##
==============================================
+ Coverage      25.15%   37.80%   +12.64%     
==============================================
  Files            119       85       -34     
  Lines           2862     1386     -1476     
  Branches         555      305      -250     
==============================================
- Hits             720      524      -196     
+ Misses          1856      695     -1161     
+ Partials         286      167      -119     

see 76 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5ff590...5377ad4. Read the comment docs.

@@ -0,0 +1,189 @@
window.config = {
Copy link
Member

Choose a reason for hiding this comment

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

can we not call this default for now? maybe say experimental_16bit

@@ -39,11 +40,27 @@ export default async function init({
configuration,
appConfig,
}: Types.Extensions.ExtensionParams): Promise<void> {
await cs3DInit();
await cs3DInit({
Copy link
Member

Choose a reason for hiding this comment

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

can you pull new changes? I added some modification to cs3DInit recently, and it makes sense to merge those changes here as well

@sedghi sedghi changed the base branch from v3-stable to master June 19, 2023 13:31
@sedghi
Copy link
Member

sedghi commented Jun 19, 2023

@Ouwen Also for this one, any one free you have to work on this?

@sedghi
Copy link
Member

sedghi commented Oct 4, 2023

merged this in another PR

@sedghi sedghi closed this Oct 4, 2023
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.

2 participants