-
Notifications
You must be signed in to change notification settings - Fork 9
add referenceManager #30
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
base: master
Are you sure you want to change the base?
Conversation
|
|
||
| namespace SeeSharp.Blazor; | ||
|
|
||
| public static class DocumentationHelper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Helper" is a bit too vague, a more descriptive name like DocumentationReader or XmlDocumentationReader would be better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A short documentation comment for this class that quickly summarizes what it does would also be nice.
|
|
||
| public static class DocumentationHelper | ||
| { | ||
| private static Dictionary<string, string> _loadedXmlDocumentation = new(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| private static Dictionary<string, string> _loadedXmlDocumentation = new(); | |
| private static Dictionary<string, string> loadedXmlDocumentation = new(); |
Convention in the code base is to not use _ prefix for private members
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is useful beyond the Blazor stuff, so maybe this file should be moved to SeeSharp/Common
| } | ||
| </div> | ||
|
|
||
| @code { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For components with a lot of code, it is generally a good idea to separate that into a code-behind file. See https://learn.microsoft.com/en-us/aspnet/core/blazor/components/?view=aspnetcore-10.0#partial-class-support
(Makes it easier to navigate and also prevents some language server quirks and bugs that unfortunately happen a lot on .razor files :( )
|
|
||
| protected override void OnInitialized() | ||
| { | ||
| integratorTypes = new Type[] {typeof(PathTracer), typeof(VertexConnectionAndMerging)}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of hard-coding these two, you should use reflections to find all non-abstract classes that inherit from Integrator
There's already a utility class for that:
| public class TypeFactory<T> where T : class { |
So TypeFactory<Integrator>.All should do the trick here ;)
| Type type = (member is PropertyInfo p) ? p.PropertyType : ((FieldInfo)member).FieldType; | ||
| Type underlyingType = Nullable.GetUnderlyingType(type) ?? type; | ||
|
|
||
| if (underlyingType == typeof(int) || underlyingType == typeof(uint)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified a bit by only considering three cases:
Integer types, floating point types, and booleans
For ints and floats, you can then use the highest precision (i.e., long and double) always for the Setting component, but convert the final result to the desired type.
(Optionally, you could also check for overflows and raise an error if the value is out-of-bounds for the lower-precision type)
| } | ||
| catch (Exception ex) | ||
| { | ||
| Console.WriteLine($"[DocHelper] Error loading XML: {ex.Message}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use the Logger class instead of Console.WriteLine. That'll make sure to color the message according to type, and show them in the UI as well
| return true; | ||
| } | ||
|
|
||
| void CopyProperties(object target, object source) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the name is misleading because it also copies the fields
| } | ||
| } | ||
|
|
||
| bool LoadIntegratorFromJson(string path) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this and the other similar (de)serialization logic should probably go into a separate helper class to make it reusable
| partialPath = Path.ChangeExtension(currentPath, "-partial.exr"); | ||
| } | ||
|
|
||
| while (currentSpp < targetTotalSpp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FrameBuffer supports continuous writing of intermediate results in exponential steps already:
SeeSharp/SeeSharp/Images/FrameBuffer.cs
Lines 125 to 129 in 3bcac11
| /// <summary> | |
| /// If set, <see cref="WriteIntermediate" /> and <see cref="WriteContinously" /> only output an image | |
| /// after each power-of-two iteration, and <see cref="SendToTev" /> is only triggered at that rate, too. | |
| /// </summary> | |
| WriteExponentially = 32, |
This should be used here
|
This update separates the logic from the UI and improves the information table display with parameter validation. |
No description provided.