-
Notifications
You must be signed in to change notification settings - Fork 264
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
Feature: improve logging related to finding files #285
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,7 +113,7 @@ struct PluginManager::PluginManagerPrivate { | |
fs::path resolved = resolver->resolve(filename); | ||
|
||
if (fs::exists(resolved)) { | ||
Log(Debug, "Loading plugin \"%s\" ..", filename.string()); | ||
Log(Debug, "Loading plugin \"%s\" from \"%s\" ..", name, resolved.string()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that's necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok - I plan to change it to use the full |
||
Plugin *plugin = new Plugin(resolved); | ||
// New classes must be registered within the class hierarchy | ||
Class::static_initialization(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,9 +13,9 @@ void spectrum_from_file(const fs::path &path, std::vector<Scalar> &wavelengths, | |
auto fs = Thread::thread()->file_resolver(); | ||
fs::path file_path = fs->resolve(path); | ||
if (!fs::exists(file_path)) | ||
Log(Error, "\"%s\": file does not exist!", file_path); | ||
Log(Error, "\"%s\": file does not exist!", file_path.string()); | ||
|
||
Log(Info, "Loading spectral data file \"%s\" ..", file_path); | ||
Log(Info, "Loading spectral data file \"%s\" ..", file_path.string()); | ||
Comment on lines
+16
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this fix a specific bug? What is the log message before and after this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually made these types of changes during a very pedantic file path review for consistency in explicitly calling |
||
std::string extension = string::to_lower(file_path.extension().string()); | ||
if (extension == ".spd") { | ||
ref<MemoryMappedFile> mmap = new MemoryMappedFile(file_path, false); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,7 +108,7 @@ class EnvironmentMapEmitter final : public Emitter<Float, Spectrum> { | |
ref<Bitmap> bitmap = new Bitmap(file_path); | ||
if (bitmap->width() < 2 || bitmap->height() < 3) | ||
Throw("\"%s\": the environment map resolution must be at " | ||
"least 2x3 pixels", m_filename); | ||
"least 2x3 pixels", file_path.string()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question here. |
||
|
||
/* Convert to linear RGBA float bitmap, will undergo further | ||
conversion into coefficients of a spectral upsampling model below */ | ||
|
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.
Shouldn't this be logged into
Trace
as well?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.
Currently
Debug
is written once per file lookup, andTrace
is written once per path check. You're right that I forgot to paste the output--maybe it makes the behavior more clear.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.
IMO this is a little too much for debug mode. I would like to be able to compile in debug mode while not always having those logs. Let's use
Trace
here to have control. In any case, the log messages you are adding in this PR are useful to debug a situation where a file couldn't be found, in which case you will need to switch toTrace
to see all the messages.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.
Ok, agree, I will make all of the outputs part of
Trace
.