From 139d2c6c6d2f4d3256a723b224c9297fe0919957 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 8 May 2021 00:44:23 +0200 Subject: [PATCH 1/8] Apply more codegen checks (#1579) * Apply more codegen checks --- ext/ghoul | 2 +- .../tasks/exoplanetsdatapreparationtask.cpp | 89 +++++-------- .../tasks/kameleondocumentationtask.cpp | 51 +++----- .../tasks/kameleonmetadatatojsontask.cpp | 50 +++---- .../tasks/kameleonvolumetorawtask.cpp | 122 +++++++----------- modules/spout/renderableplanespout.cpp | 42 ++---- modules/spout/screenspacespout.cpp | 42 ++---- modules/sync/tasks/syncassettask.cpp | 37 ++---- .../rendering/renderabletimevaryingvolume.cpp | 84 +++++------- src/scene/scale.cpp | 34 ++--- src/scene/timeframe.cpp | 38 ++---- src/util/task.cpp | 39 +++--- src/util/timerange.cpp | 49 +++---- support/coding/codegen | 2 +- 14 files changed, 239 insertions(+), 442 deletions(-) diff --git a/ext/ghoul b/ext/ghoul index 3c0d660924..324db538a6 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit 3c0d660924ea66ab65703cba7f794193c3c17d67 +Subproject commit 324db538a63653fa9bf6d78cf04135ece0fa3c3f diff --git a/modules/exoplanets/tasks/exoplanetsdatapreparationtask.cpp b/modules/exoplanets/tasks/exoplanetsdatapreparationtask.cpp index 837e9cafa3..20f6697e50 100644 --- a/modules/exoplanets/tasks/exoplanetsdatapreparationtask.cpp +++ b/modules/exoplanets/tasks/exoplanetsdatapreparationtask.cpp @@ -34,34 +34,50 @@ #include #include #include +#include #include namespace { - constexpr const char* KeyInputDataFile = "InputDataFile"; - constexpr const char* KeyInputSpeck = "InputSPECK"; - constexpr const char* KeyOutputBin = "OutputBIN"; - constexpr const char* KeyOutputLut = "OutputLUT"; - constexpr const char* KeyTeffToBv = "TeffToBvFile"; - constexpr const char* _loggerCat = "ExoplanetsDataPreparationTask"; + + struct [[codegen::Dictionary(ExoplanetsDataPreparationTask)]] Parameters { + // The csv file to extract data from + std::filesystem::path inputDataFile; + + // The speck file with star locations + std::filesystem::path inputSPECK; + + // The bin file to export data into + std::string outputBIN [[codegen::annotation("A valid filepath")]]; + + // The txt file to write look-up table into + std::string outputLUT [[codegen::annotation("A valid filepath")]]; + + // The path to a teff to bv conversion file. Should be a txt file where each line + // has the format 'teff,bv' + std::filesystem::path teffToBvFile; + }; +#include "exoplanetsdatapreparationtask_codegen.cpp" } // namespace namespace openspace::exoplanets { +documentation::Documentation ExoplanetsDataPreparationTask::documentation() { + documentation::Documentation doc = codegen::doc(); + doc.id = "exoplanets_data_preparation_task"; + return doc; +} + ExoplanetsDataPreparationTask::ExoplanetsDataPreparationTask( const ghoul::Dictionary& dictionary) { - openspace::documentation::testSpecificationAndThrow( - documentation(), - dictionary, - "ExoplanetsDataPreparationTask" - ); + const Parameters p = codegen::bake(dictionary); - _inputDataPath = absPath(dictionary.value(KeyInputDataFile)); - _inputSpeckPath = absPath(dictionary.value(KeyInputSpeck)); - _outputBinPath = absPath(dictionary.value(KeyOutputBin)); - _outputLutPath = absPath(dictionary.value(KeyOutputLut)); - _teffToBvFilePath = absPath(dictionary.value(KeyTeffToBv)); + _inputDataPath = absPath(p.inputDataFile.string()); + _inputSpeckPath = absPath(p.inputSPECK.string()); + _outputBinPath = absPath(p.outputBIN); + _outputLutPath = absPath(p.outputLUT); + _teffToBvFilePath = absPath(p.teffToBvFile.string()); } std::string ExoplanetsDataPreparationTask::description() { @@ -441,45 +457,4 @@ float ExoplanetsDataPreparationTask::bvFromTeff(float teff) { return bv; } -documentation::Documentation ExoplanetsDataPreparationTask::documentation() { - using namespace documentation; - return { - "ExoplanetsDataPreparationTask", - "exoplanets_data_preparation_task", - { - { - KeyInputDataFile, - new FileVerifier, - Optional::No, - "The csv file to extract data from" - }, - { - KeyInputSpeck, - new FileVerifier, - Optional::No, - "The speck file with star locations" - }, - { - KeyOutputBin, - new StringAnnotationVerifier("A valid filepath"), - Optional::No, - "The bin file to export data into" - }, - { - KeyOutputLut, - new StringAnnotationVerifier("A valid filepath"), - Optional::No, - "The txt file to write look-up table into" - }, - { - KeyTeffToBv, - new FileVerifier, - Optional::No, - "The path to a teff to bv conversion file. Should be a txt file where " - "each line has the format 'teff,bv'" - } - } - }; -} - } // namespace openspace::exoplanets diff --git a/modules/kameleonvolume/tasks/kameleondocumentationtask.cpp b/modules/kameleonvolume/tasks/kameleondocumentationtask.cpp index 9e4cac12a5..3e3a3f0178 100644 --- a/modules/kameleonvolume/tasks/kameleondocumentationtask.cpp +++ b/modules/kameleonvolume/tasks/kameleondocumentationtask.cpp @@ -29,30 +29,39 @@ #include #include #include +#include #include namespace { - constexpr const char* KeyInput = "Input"; - constexpr const char* KeyOutput = "Output"; constexpr const char* MainTemplateFilename = "${WEB}/kameleondocumentation/main.hbs"; constexpr const char* HandlebarsFilename = "${WEB}/common/handlebars-v4.0.5.js"; constexpr const char* JsFilename = "${WEB}/kameleondocumentation/script.js"; constexpr const char* BootstrapFilename = "${WEB}/common/bootstrap.min.css"; constexpr const char* CssFilename = "${WEB}/common/style.css"; + + struct [[codegen::Dictionary(KameleonDocumentationTask)]] Parameters { + // The CDF file to extract data from + std::filesystem::path input; + + // The HTML file to write documentation to + std::string output [[codegen::annotation("A valid filepath")]]; + }; +#include "kameleondocumentationtask_codegen.cpp" } // namespace namespace openspace::kameleonvolume { +documentation::Documentation KameleonDocumentationTask::documentation() { + documentation::Documentation doc = codegen::doc(); + doc.id = "kameleon_documentation_task"; + return doc; +} + KameleonDocumentationTask::KameleonDocumentationTask(const ghoul::Dictionary& dictionary) { - openspace::documentation::testSpecificationAndThrow( - documentation(), - dictionary, - "KameleonDocumentationTask" - ); - - _inputPath = absPath(dictionary.value(KeyInput)); - _outputPath = absPath(dictionary.value(KeyOutput)); + const Parameters p = codegen::bake(dictionary); + _inputPath = absPath(p.input.string()); + _outputPath = absPath(p.output); } std::string KameleonDocumentationTask::description() { @@ -146,26 +155,4 @@ void KameleonDocumentationTask::perform(const Task::ProgressCallback & progressC progressCallback(1.0f); } -documentation::Documentation KameleonDocumentationTask::documentation() { - using namespace documentation; - return { - "KameleonDocumentationTask", - "kameleon_documentation_task", - { - { - KeyInput, - new StringAnnotationVerifier("A file path to a cdf file"), - Optional::No, - "The cdf file to extract data from" - }, - { - KeyOutput, - new StringAnnotationVerifier("A valid filepath"), - Optional::No, - "The html file to write documentation to" - } - } - }; -} - } // namespace openspace::kameleonvolume diff --git a/modules/kameleonvolume/tasks/kameleonmetadatatojsontask.cpp b/modules/kameleonvolume/tasks/kameleonmetadatatojsontask.cpp index ecd1bcccf5..4442a0b15e 100644 --- a/modules/kameleonvolume/tasks/kameleonmetadatatojsontask.cpp +++ b/modules/kameleonvolume/tasks/kameleonmetadatatojsontask.cpp @@ -29,26 +29,34 @@ #include #include #include +#include #include namespace { - constexpr const char* KeyInput = "Input"; - constexpr const char* KeyOutput = "Output"; + struct [[codegen::Dictionary(KameleonMetadataToJsonTask)]] Parameters { + // The CDF file to extract data from + std::filesystem::path input; + + // The JSON file to export data into + std::string output [[codegen::annotation("A valid filepath")]]; + }; +#include "kameleonmetadatatojsontask_codegen.cpp" } // namespace namespace openspace::kameleonvolume { +documentation::Documentation KameleonMetadataToJsonTask::documentation() { + documentation::Documentation doc = codegen::doc(); + doc.id = "kameleon_metadata_to_json_task"; + return doc; +} + KameleonMetadataToJsonTask::KameleonMetadataToJsonTask( const ghoul::Dictionary& dictionary) { - openspace::documentation::testSpecificationAndThrow( - documentation(), - dictionary, - "KameleonMetadataToJsonTask" - ); - - _inputPath = absPath(dictionary.value(KeyInput)); - _outputPath = absPath(dictionary.value(KeyOutput)); + const Parameters p = codegen::bake(dictionary); + _inputPath = absPath(p.input.string()); + _outputPath = absPath(p.output); } std::string KameleonMetadataToJsonTask::description() { @@ -69,26 +77,4 @@ void KameleonMetadataToJsonTask::perform(const Task::ProgressCallback& progressC progressCallback(1.0f); } -documentation::Documentation KameleonMetadataToJsonTask::documentation() { - using namespace documentation; - return { - "KameleonMetadataToJsonTask", - "kameleon_metadata_to_json_task", - { - { - KeyInput, - new StringAnnotationVerifier("A file path to a cdf file"), - Optional::No, - "The cdf file to extract data from" - }, - { - KeyOutput, - new StringAnnotationVerifier("A valid filepath"), - Optional::No, - "The JSON file to export data into" - } - } - }; -} - } // namespace openspace::kameleonvolume diff --git a/modules/kameleonvolume/tasks/kameleonvolumetorawtask.cpp b/modules/kameleonvolume/tasks/kameleonvolumetorawtask.cpp index fd9c342afe..c7f6eb037a 100644 --- a/modules/kameleonvolume/tasks/kameleonvolumetorawtask.cpp +++ b/modules/kameleonvolume/tasks/kameleonvolumetorawtask.cpp @@ -31,13 +31,11 @@ #include #include #include +#include +#include namespace { - constexpr const char* KeyInput = "Input"; - constexpr const char* KeyRawVolumeOutput = "RawVolumeOutput"; - constexpr const char* KeyDictionaryOutput = "DictionaryOutput"; constexpr const char* KeyDimensions = "Dimensions"; - constexpr const char* KeyVariable = "Variable"; constexpr const char* KeyTime = "Time"; constexpr const char* KeyLowerDomainBound = "LowerDomainBound"; constexpr const char* KeyUpperDomainBound = "UpperDomainBound"; @@ -46,92 +44,64 @@ namespace { constexpr const char* KeyMaxValue = "MaxValue"; constexpr const char* KeyVisUnit = "VisUnit"; + + struct [[codegen::Dictionary(KameleonVolumeToRawTask)]] Parameters { + // The cdf file to extract data from + std::filesystem::path input; + + // The raw volume file to export data to + std::string rawVolumeOutput [[codegen::annotation("A valid filepath")]]; + + // The Lua dictionary file to export metadata to + std::string dictionaryOutput [[codegen::annotation("A valid filepath")]]; + + // The variable name to read from the kameleon dataset + std::string variable [[codegen::annotation("A valid kameleon variable")]]; + + // A vector representing the number of cells in each dimension + glm::ivec3 dimensions; + + // A vector representing the lower bound of the domain, in the native kameleon + // grid units + std::optional lowerDomainBound; + + // A vector representing the lower bound of the domain, in the native kameleon + // grid units + std::optional upperDomainBound; + + // The unit of the data + std::optional visUnit + [[codegen::annotation("A valid kameleon unit")]]; + }; +#include "kameleonvolumetorawtask_codegen.cpp" } // namespace namespace openspace::kameleonvolume { documentation::Documentation KameleonVolumeToRawTask::documentation() { - using namespace documentation; - return { - "KameleonVolumeToRawTask", - "kameleon_metadata_to_json_task", - { - { - KeyInput, - new StringAnnotationVerifier("A file path to a cdf file"), - Optional::No, - "The cdf file to extract data from", - }, - { - KeyRawVolumeOutput, - new StringAnnotationVerifier("A valid filepath"), - Optional::No, - "The raw volume file to export data to", - }, - { - KeyDictionaryOutput, - new StringAnnotationVerifier("A valid filepath"), - Optional::No, - "The lua dictionary file to export metadata to", - }, - { - KeyVariable, - new StringAnnotationVerifier("A valid kameleon variable"), - Optional::No, - "The variable name to read from the kameleon dataset", - }, - { - KeyDimensions, - new DoubleVector3Verifier, - Optional::No, - "A vector representing the number of cells in each dimension", - }, - { - KeyLowerDomainBound, - new DoubleVector3Verifier, - Optional::Yes, - "A vector representing the lower bound of the domain, " - "in the native kameleon grid units", - }, - { - KeyUpperDomainBound, - new DoubleVector3Verifier, - Optional::Yes, - "A vector representing the lower bound of the domain, " - "in the native kameleon grid units" - }, - { - KeyVisUnit, - new StringAnnotationVerifier("A valid kameleon unit"), - Optional::Yes, - "The unit of the data", - } - } - }; + documentation::Documentation doc = codegen::doc(); + doc.id = "kameleon_metadata_to_json_task"; + return doc; } - KameleonVolumeToRawTask::KameleonVolumeToRawTask(const ghoul::Dictionary& dictionary) { - openspace::documentation::testSpecificationAndThrow( - documentation(), - dictionary, - "KameleonVolumeToRawTask" - ); + const Parameters p = codegen::bake(dictionary); - _inputPath = absPath(dictionary.value(KeyInput)); - _rawVolumeOutputPath = absPath(dictionary.value(KeyRawVolumeOutput)); - _dictionaryOutputPath = absPath(dictionary.value(KeyDictionaryOutput)); - _variable = dictionary.value(KeyVariable); - _dimensions = glm::uvec3(dictionary.value(KeyDimensions)); + _inputPath = absPath(p.input.string()); + _rawVolumeOutputPath = absPath(p.rawVolumeOutput); + _dictionaryOutputPath = absPath(p.dictionaryOutput); + _variable = p.variable; + _dimensions = p.dimensions; - if (dictionary.hasKey(KeyLowerDomainBound)) { - _lowerDomainBound = dictionary.value(KeyLowerDomainBound); + if (p.lowerDomainBound.has_value()) { + _lowerDomainBound = *p.lowerDomainBound; } else { _autoDomainBounds = true; } - if (dictionary.hasKey(KeyUpperDomainBound)) { - _upperDomainBound = dictionary.value(KeyUpperDomainBound); + + if (p.upperDomainBound.has_value()) { + _upperDomainBound = *p.upperDomainBound; } else { _autoDomainBounds = true; diff --git a/modules/spout/renderableplanespout.cpp b/modules/spout/renderableplanespout.cpp index 12e539409f..1344dea70f 100644 --- a/modules/spout/renderableplanespout.cpp +++ b/modules/spout/renderableplanespout.cpp @@ -29,12 +29,11 @@ #include #include #include +#include namespace { constexpr const char* LoggerCat = "ScreenSpaceSpout"; - constexpr const char* KeyName = "Name"; - constexpr openspace::properties::Property::PropertyInfo NameInfo = { "SpoutName", "Spout Sender Name", @@ -56,30 +55,20 @@ namespace { "If this property is trigged, the 'SpoutSelection' options will be refreshed." }; + struct [[codegen::Dictionary(RenderablePlaneSpout)]] Parameters { + // [[codegen::verbatim(NameInfo.description)]] + std::optional spoutName; + }; +#include "renderableplanespout_codegen.cpp" + } // namespace namespace openspace { documentation::Documentation RenderablePlaneSpout::Documentation() { - using namespace openspace::documentation; - return { - "ScreenSpace Spout", - "spout_screenspace_spout", - { - { - KeyName, - new StringVerifier, - Optional::Yes, - "Specifies the GUI name of the ScreenspaceSpout" - }, - { - NameInfo.identifier, - new StringVerifier, - Optional::Yes, - NameInfo.description - } - } - }; + documentation::Documentation doc = codegen::doc(); + doc.id = "spout_screenspace_spout"; + return doc; } RenderablePlaneSpout::RenderablePlaneSpout(const ghoul::Dictionary& dictionary) @@ -89,11 +78,7 @@ RenderablePlaneSpout::RenderablePlaneSpout(const ghoul::Dictionary& dictionary) , _updateSelection(UpdateInfo) , _receiver(GetSpout()) { - documentation::testSpecificationAndThrow( - Documentation(), - dictionary, - "RenderablePlaneSpout" - ); + const Parameters p = codegen::bake(dictionary); int iIdentifier = 0; if (_identifier.empty()) { @@ -114,10 +99,7 @@ RenderablePlaneSpout::RenderablePlaneSpout(const ghoul::Dictionary& dictionary) setGuiName("RenderablePlaneSpout " + std::to_string(iIdentifier)); } - if (dictionary.hasKey(NameInfo.identifier)) { - _spoutName = dictionary.value(NameInfo.identifier); - } - + _spoutName = p.spoutName.value_or(_spoutName); _spoutName.onChange([this]() { _isSpoutDirty = true; _isErrorMessageDisplayed = false; diff --git a/modules/spout/screenspacespout.cpp b/modules/spout/screenspacespout.cpp index b47d28dff3..4265a4ff7c 100644 --- a/modules/spout/screenspacespout.cpp +++ b/modules/spout/screenspacespout.cpp @@ -29,10 +29,9 @@ #include #include #include +#include namespace { - constexpr const char* KeyName = "Name"; - constexpr openspace::properties::Property::PropertyInfo NameInfo = { "SpoutName", "Spout Sender Name", @@ -54,30 +53,20 @@ namespace { "If this property is trigged, the 'SpoutSelection' options will be refreshed." }; + struct [[codegen::Dictionary(ScreenSpaceSpout)]] Parameters { + // [[codegen::verbatim(NameInfo.description)]] + std::optional spoutName; + }; +#include "screenspacespout_codegen.cpp" + } // namespace namespace openspace { documentation::Documentation ScreenSpaceSpout::Documentation() { - using namespace openspace::documentation; - return { - "ScreenSpace Spout", - "spout_screenspace_spout", - { - { - KeyName, - new StringVerifier, - Optional::Yes, - "Specifies the GUI name of the ScreenspaceSpout" - }, - { - NameInfo.identifier, - new StringVerifier, - Optional::Yes, - NameInfo.description - } - } - }; + documentation::Documentation doc = codegen::doc(); + doc.id = "spout_screenspace_spout"; + return doc; } ScreenSpaceSpout::ScreenSpaceSpout(const ghoul::Dictionary& dictionary) @@ -87,11 +76,7 @@ ScreenSpaceSpout::ScreenSpaceSpout(const ghoul::Dictionary& dictionary) , _updateSelection(UpdateInfo) , _receiver(GetSpout()) { - documentation::testSpecificationAndThrow( - Documentation(), - dictionary, - "ScreenSpaceSpout" - ); + const Parameters p = codegen::bake(dictionary); std::string identifier; if (dictionary.hasValue(KeyIdentifier)) { @@ -103,10 +88,7 @@ ScreenSpaceSpout::ScreenSpaceSpout(const ghoul::Dictionary& dictionary) identifier = makeUniqueIdentifier(identifier); setIdentifier(std::move(identifier)); - if (dictionary.hasKey(NameInfo.identifier)) { - _spoutName = dictionary.value(NameInfo.identifier); - } - + _spoutName = p.spoutName.value_or(_spoutName); _spoutName.onChange([this]() { _isSpoutDirty = true; _isErrorMessageDisplayed = false; diff --git a/modules/sync/tasks/syncassettask.cpp b/modules/sync/tasks/syncassettask.cpp index b670301dff..5c15a7d3e0 100644 --- a/modules/sync/tasks/syncassettask.cpp +++ b/modules/sync/tasks/syncassettask.cpp @@ -37,43 +37,34 @@ #include #include -#include -#include #include - +#include +#include +#include #include namespace { constexpr const char* KeyAsset = "Asset"; constexpr std::chrono::milliseconds ProgressPollInterval(200); + + struct [[codegen::Dictionary(SyncAssetTask)]] Parameters { + // The asset file to sync + std::filesystem::path asset; + }; +#include "syncassettask_codegen.cpp" } // namespace namespace openspace { documentation::Documentation SyncAssetTask::documentation() { - using namespace documentation; - return { - "SyncAssetTask", - "sync_asset_task", - { - { - KeyAsset, - new StringAnnotationVerifier("A file path to an asset"), - Optional::No, - "The asset file to sync" - } - } - }; + documentation::Documentation doc = codegen::doc(); + doc.id = "sync_asset_task"; + return doc; } SyncAssetTask::SyncAssetTask(const ghoul::Dictionary& dictionary) { - documentation::testSpecificationAndThrow( - documentation(), - dictionary, - "SyncAssetTask" - ); - - _asset = dictionary.value(KeyAsset); + const Parameters p = codegen::bake(dictionary); + _asset = p.asset.string(); } std::string SyncAssetTask::description() { diff --git a/modules/volume/rendering/renderabletimevaryingvolume.cpp b/modules/volume/rendering/renderabletimevaryingvolume.cpp index fc32f26bc9..71494ca0ab 100644 --- a/modules/volume/rendering/renderabletimevaryingvolume.cpp +++ b/modules/volume/rendering/renderabletimevaryingvolume.cpp @@ -44,6 +44,7 @@ #include #include #include +#include namespace { constexpr const char* _loggerCat = "RenderableTimeVaryingVolume"; @@ -54,11 +55,6 @@ namespace { const char* KeyStepSize = "StepSize"; const char* KeyGridType = "GridType"; const char* KeyTransferFunction = "TransferFunction"; - const char* KeySourceDirectory = "SourceDirectory"; - - const char* KeyClipPlanes = "ClipPlanes"; - const char* KeySecondsBefore = "SecondsBefore"; - const char* KeySecondsAfter = "SecondsAfter"; const float SecondsInOneDay = 60 * 60 * 24; constexpr const float VolumeMaxOpacity = 500; @@ -129,44 +125,33 @@ namespace { "Radius upper bound", "" // @TODO Missing documentation }; + + struct [[codegen::Dictionary(RenderableTimeVaryingVolume)]] Parameters { + // Specifies the path to load timesteps from + std::string sourceDirectory; + + // Specifies the transfer function file path + std::string transferFunction; + + // Specifies the number of seconds to show the the first timestep before its + // actual time. The default value is 0 + std::optional secondsBefore; + + // Specifies the number of seconds to show the the last timestep after its + // actual time + float secondsAfter; + + std::optional clipPlanes; + }; +#include "renderabletimevaryingvolume_codegen.cpp" } // namespace namespace openspace::volume { - documentation::Documentation RenderableTimeVaryingVolume::Documentation() { - using namespace documentation; - return { - "RenderableTimevaryingVolume", - "volume_renderable_timevaryingvolume", - { - { - KeySourceDirectory, - new StringVerifier, - Optional::No, - "Specifies the path to load timesteps from" - }, - { - KeyTransferFunction, - new StringVerifier, - Optional::No, - "Specifies the transfer function file path" - }, - { - KeySecondsBefore, - new DoubleVerifier, - Optional::Yes, - "Specifies the number of seconds to show the the first timestep before " - "its actual time. The default value is 0." - }, - { - KeySecondsAfter, - new DoubleVerifier, - Optional::No, - "Specifies the number of seconds to show the the last timestep after its " - "actual time" - } - } - }; +documentation::Documentation RenderableTimeVaryingVolume::Documentation() { + documentation::Documentation doc = codegen::doc(); + doc.id = "volume_renderable_timevaryingvolume"; + return doc; } RenderableTimeVaryingVolume::RenderableTimeVaryingVolume( @@ -184,14 +169,10 @@ RenderableTimeVaryingVolume::RenderableTimeVaryingVolume( , _jumpToTimestep(JumpToTimestepInfo, 0, 0, 256) , _currentTimestep(CurrentTimeStepInfo, 0, 0, 256) { - documentation::testSpecificationAndThrow( - Documentation(), - dictionary, - "RenderableTimeVaryingVolume" - ); + const Parameters p = codegen::bake(dictionary); - _sourceDirectory = absPath(dictionary.value(KeySourceDirectory)); - _transferFunctionPath = absPath(dictionary.value(KeyTransferFunction)); + _sourceDirectory = absPath(p.sourceDirectory); + _transferFunctionPath = absPath(p.transferFunction); _transferFunction = std::make_shared( _transferFunctionPath, [](const openspace::TransferFunction&) {} @@ -207,15 +188,10 @@ RenderableTimeVaryingVolume::RenderableTimeVaryingVolume( _stepSize = static_cast(dictionary.value(KeyStepSize)); } - if (dictionary.hasValue(KeySecondsBefore)) { - _secondsBefore = static_cast(dictionary.value(KeySecondsBefore)); - } - _secondsAfter = static_cast(dictionary.value(KeySecondsAfter)); + _secondsBefore = p.secondsBefore.value_or(_secondsBefore); + _secondsAfter = p.secondsAfter; - ghoul::Dictionary clipPlanesDictionary; - if (dictionary.hasValue(KeyClipPlanes)) { - clipPlanesDictionary.value(KeyClipPlanes); - } + ghoul::Dictionary clipPlanesDictionary = p.clipPlanes.value_or(ghoul::Dictionary()); _clipPlanes = std::make_shared(clipPlanesDictionary); _clipPlanes->setIdentifier("clipPlanes"); _clipPlanes->setGuiName("Clip Planes"); diff --git a/src/scene/scale.cpp b/src/scene/scale.cpp index b22af536a8..9ee249066a 100644 --- a/src/scene/scale.cpp +++ b/src/scene/scale.cpp @@ -35,41 +35,31 @@ #include namespace { - constexpr const char* KeyType = "Type"; + struct [[codegen::Dictionary(Scale)]] Parameters { + // The type of the scaling that is described in this element. The available types + // of scaling depend on the configuration of the application and can be written to + // disk on application startup into the FactoryDocumentation + std::string type [[codegen::annotation("Must name a valid Scale type")]]; + }; +#include "scale_codegen.cpp" } // namespace namespace openspace { documentation::Documentation Scale::Documentation() { - using namespace openspace::documentation; - - return { - "Transformation Scaling", - "core_transform_scaling", - { - { - KeyType, - new StringAnnotationVerifier("Must name a valid Scale type"), - Optional::No, - "The type of the scaling that is described in this element. " - "The available types of scaling depend on the configuration " - "of the application and can be written to disk on " - "application startup into the FactoryDocumentation." - } - } - }; + documentation::Documentation doc = codegen::doc(); + doc.id = "core_transform_scaling"; + return doc; } ghoul::mm_unique_ptr Scale::createFromDictionary( const ghoul::Dictionary& dictionary) { - documentation::testSpecificationAndThrow(Documentation(), dictionary, "Scale"); - - std::string scaleType = dictionary.value(KeyType); + const Parameters p = codegen::bake(dictionary); auto factory = FactoryManager::ref().factory(); Scale* result = factory->create( - scaleType, + p.type, dictionary, &global::memoryManager->PersistentMemory ); diff --git a/src/scene/timeframe.cpp b/src/scene/timeframe.cpp index 5ecebb4f62..ccf380c2c0 100644 --- a/src/scene/timeframe.cpp +++ b/src/scene/timeframe.cpp @@ -35,44 +35,30 @@ #include namespace { - constexpr const char* KeyType = "Type"; + struct [[codegen::Dictionary(TimeFrame)]] Parameters { + // The type of the time frame that is described in this element. The available + // types of scaling depend on the configuration of the application and can be + // written to disk on application startup into the FactoryDocumentation + std::string type [[codegen::annotation("Must name a valid TimeFrame type")]]; + }; +#include "timeframe_codegen.cpp" } // namespace namespace openspace { documentation::Documentation TimeFrame::Documentation() { - using namespace openspace::documentation; - - return { - "Time Frame", - "core_time_frame", - { - { - KeyType, - new StringAnnotationVerifier("Must name a valid TimeFrame type"), - Optional::No, - "The type of the time frame that is described in this element. " - "The available types of scaling depend on the configuration " - "of the application and can be written to disk on " - "application startup into the FactoryDocumentation." - } - } - }; + documentation::Documentation doc = codegen::doc(); + doc.id = "core_time_frame"; + return doc; } ghoul::mm_unique_ptr TimeFrame::createFromDictionary( const ghoul::Dictionary& dictionary) { - documentation::testSpecificationAndThrow(Documentation(), dictionary, "TimeFrame"); - - const std::string timeFrameType = dictionary.value(KeyType); + const Parameters p = codegen::bake(dictionary); auto factory = FactoryManager::ref().factory(); - TimeFrame* result = factory->create( - timeFrameType, - dictionary/*, - &global::memoryManager.PersistentMemory*/ - ); + TimeFrame* result = factory->create(p.type, dictionary); result->setIdentifier("TimeFrame"); return ghoul::mm_unique_ptr(result); } diff --git a/src/util/task.cpp b/src/util/task.cpp index 23dac7fc42..e775dca720 100644 --- a/src/util/task.cpp +++ b/src/util/task.cpp @@ -30,38 +30,29 @@ #include #include +namespace { + struct [[codegen::Dictionary(Task)]] Parameters { + // This key specifies the type of Task that gets created. It has to be one of the + // valid Tasks that are available for creation (see the FactoryDocumentation for a + // list of possible Tasks), which depends on the configration of the application + std::string type [[codegen::annotation("A valid Task created by a factory")]]; + }; +#include "task_codegen.cpp" +} // namespace + namespace openspace { documentation::Documentation Task::documentation() { - using namespace documentation; - return { - "Task", - "core_task", - { - { - "Type", - new StringAnnotationVerifier("A valid Task created by a factory"), - Optional::No, - "This key specifies the type of Task that gets created. It has to be one" - "of the valid Tasks that are available for creation (see the " - "FactoryDocumentation for a list of possible Tasks), which depends on " - "the configration of the application" - } - } - }; + documentation::Documentation doc = codegen::doc(); + doc.id = "core_task"; + return doc; } std::unique_ptr Task::createFromDictionary(const ghoul::Dictionary& dictionary) { - openspace::documentation::testSpecificationAndThrow( - documentation::Documentation(), - dictionary, - "Task" - ); + const Parameters p = codegen::bake(dictionary); - std::string taskType = dictionary.value("Type"); auto factory = FactoryManager::ref().factory(); - - Task* task = factory->create(taskType, dictionary); + Task* task = factory->create(p.type, dictionary); return std::unique_ptr(task); } diff --git a/src/util/timerange.cpp b/src/util/timerange.cpp index 443cc2eca9..f56b2e1b41 100644 --- a/src/util/timerange.cpp +++ b/src/util/timerange.cpp @@ -31,32 +31,22 @@ #include namespace { - constexpr const char* KeyStart = "Start"; - constexpr const char* KeyEnd = "End"; + struct [[codegen::Dictionary(TimeRange)]] Parameters { + // The start date of the time range + std::string start [[codegen::annotation("A string representing a valid date")]]; + + // The end date of the time range + std::string end [[codegen::annotation("A string representing a valid date")]]; + }; +#include "timerange_codegen.cpp" } // namespace namespace openspace { documentation::Documentation TimeRange::Documentation() { - using namespace documentation; - return { - "Time Range", - "core_util_timerange", - { - { - KeyStart, - new StringAnnotationVerifier("A string representing a valid date"), - Optional::No, - "The start date of the time range" - }, - { - KeyEnd, - new StringAnnotationVerifier("A string representing a valid date"), - Optional::No, - "The end date of the time range" - } - } - }; + documentation::Documentation doc = codegen::doc(); + doc.id = "core_util_timerange"; + return doc; } TimeRange::TimeRange(double startTime, double endTime) @@ -73,19 +63,10 @@ TimeRange::TimeRange(const ghoul::Dictionary& dict) { bool TimeRange::initializeFromDictionary(const ghoul::Dictionary& dict, TimeRange& timeRange) { - if (dict.hasValue(KeyStart) && dict.hasValue(KeyEnd)) { - std::string startTimeStr = dict.value(KeyStart); - std::string endTimeStr = dict.value(KeyEnd); - // Parse to date. - // @TODO converting string to time stamp should not rely on Spice - timeRange.start = SpiceManager::ref().ephemerisTimeFromDate(startTimeStr); - timeRange.end = SpiceManager::ref().ephemerisTimeFromDate(endTimeStr); - return true; - } - else { - // Could not read TimeRange from Dict - return false; - } + const Parameters p = codegen::bake(dict); + timeRange.start = SpiceManager::ref().ephemerisTimeFromDate(p.start); + timeRange.end = SpiceManager::ref().ephemerisTimeFromDate(p.end); + return true; } void TimeRange::include(double val) { diff --git a/support/coding/codegen b/support/coding/codegen index 4b06f1ad45..481f12fd98 160000 --- a/support/coding/codegen +++ b/support/coding/codegen @@ -1 +1 @@ -Subproject commit 4b06f1ad4586289f0f1523915e11b1c81fe21202 +Subproject commit 481f12fd987596851324ff89fd981df75127789d From 54fc94bfaf527b75234c9faf496c50f5a5506bef Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 8 May 2021 01:18:50 +0200 Subject: [PATCH 2/8] Remove unused parameters and document the remaining properties of RenderableGlobe (closes #1470) --- modules/globebrowsing/src/renderableglobe.cpp | 122 ++++-------------- modules/globebrowsing/src/renderableglobe.h | 4 - 2 files changed, 27 insertions(+), 99 deletions(-) diff --git a/modules/globebrowsing/src/renderableglobe.cpp b/modules/globebrowsing/src/renderableglobe.cpp index 39c561518f..7f5b65e2da 100644 --- a/modules/globebrowsing/src/renderableglobe.cpp +++ b/modules/globebrowsing/src/renderableglobe.cpp @@ -107,62 +107,56 @@ namespace { constexpr openspace::properties::Property::PropertyInfo ShowChunkEdgeInfo = { "ShowChunkEdges", "Show chunk edges", - "" // @TODO Missing documentation - }; - - constexpr openspace::properties::Property::PropertyInfo ShowChunkBoundsInfo = { - "ShowChunkBounds", - "Show chunk bounds", - "" // @TODO Missing documentation - }; - - constexpr openspace::properties::Property::PropertyInfo HeightResolutionInfo = { - "ShowHeightResolution", - "Show height resolution", - "" // @TODO Missing documentation - }; - - constexpr openspace::properties::Property::PropertyInfo HeightIntensityInfo = { - "ShowHeightIntensities", - "Show height intensities", - "" // @TODO Missing documentation + "If this value is set to 'true', the borders between chunks are shown using a " + "red highlight" }; constexpr openspace::properties::Property::PropertyInfo LevelProjectedAreaInfo = { "LevelByProjectedAreaElseDistance", "Level by projected area (else distance)", - "" // @TODO Missing documentation + "If this value is set to 'true', the tile level is determined by the area " + "projected on screen. If it is 'false', the distance to the center of the tile " + "is used instead." }; constexpr openspace::properties::Property::PropertyInfo ResetTileProviderInfo = { "ResetTileProviders", "Reset tile providers", - "" // @TODO Missing documentation + "If this property is triggered, all tile provides for the globe are reset and " + "data is reloaded from scratch." }; constexpr openspace::properties::Property::PropertyInfo ModelSpaceRenderingInfo = { "ModelSpaceRenderingCutoffLevel", "Model Space Rendering Cutoff Level", - "" // @TODO Missing documentation + "This value determines the tile level that is used as the cut off between " + "rendering tiles using the globe model rendering vs the flat in-game rendering " + "method. This value is a tradeoff between not having precision errors in the " + "rendering and represting a tile as flat or curved." }; constexpr openspace::properties::Property::PropertyInfo DynamicLodIterationCountInfo = { "DynamicLodIterationCount", "Data availability checks before LOD factor impact", - "" // @TODO Missing documentation + "The number of checks that have to fail/succeed in a row before the dynamic " + "level-of-detail adjusts the actual level-of-detail up or down" }; constexpr openspace::properties::Property::PropertyInfo PerformShadingInfo = { "PerformShading", "Perform shading", - "" // @TODO Missing documentation + "This value determines whether there should be lighting applied to the surface " + "of the globe. Note that if there is an atmosphere attached to the planet, there " + "is a separate setting to control the shadowing induced by the atmosphere part." }; constexpr openspace::properties::Property::PropertyInfo AccurateNormalsInfo = { "UseAccurateNormals", "Use Accurate Normals", - "" // @TODO Missing documentation + "This value determines whether higher-accuracy normals should be used in the " + "rendering. These normals are calculated based on the height field information " + "and are thus only available if the planet has a height map" }; constexpr openspace::properties::Property::PropertyInfo EclipseInfo = { @@ -200,25 +194,22 @@ namespace { constexpr openspace::properties::Property::PropertyInfo TargetLodScaleFactorInfo = { "TargetLodScaleFactor", "Target Level of Detail Scale Factor", - "" // @TODO Missing documentation + "Determines the targeted level-of-detail of the tiles for this globe. A higher " + "value means that the tiles rendered are a higher resolution for the same " + "distance of the camera to the planet." }; constexpr openspace::properties::Property::PropertyInfo CurrentLodScaleFactorInfo = { "CurrentLodScaleFactor", "Current Level of Detail Scale Factor (Read Only)", - "" // @TODO Missing documentation - }; - - constexpr openspace::properties::Property::PropertyInfo CameraMinHeightInfo = { - "CameraMinHeight", - "Camera Minimum Height", - "" // @TODO Missing documentation + "The currently used scale factor whose target value is deteremined by " + "'TargetLodScaleFactor'." }; constexpr openspace::properties::Property::PropertyInfo OrenNayarRoughnessInfo = { "OrenNayarRoughness", "orenNayarRoughness", - "" // @TODO Missing documentation + "The roughness factor that is used for the Oren-Nayar lighting mode" }; constexpr openspace::properties::Property::PropertyInfo NActiveLayersInfo = { @@ -506,9 +497,6 @@ RenderableGlobe::RenderableGlobe(const ghoul::Dictionary& dictionary) : Renderable(dictionary) , _debugProperties({ BoolProperty(ShowChunkEdgeInfo, false), - BoolProperty(ShowChunkBoundsInfo, false), - BoolProperty(HeightResolutionInfo, false), - BoolProperty(HeightIntensityInfo, false), BoolProperty(LevelProjectedAreaInfo, true), BoolProperty(ResetTileProviderInfo, false), IntProperty(ModelSpaceRenderingInfo, 14, 1, 22), @@ -524,7 +512,6 @@ RenderableGlobe::RenderableGlobe(const ghoul::Dictionary& dictionary) IntProperty(NumberShadowSamplesInfo, 5, 1, 7), FloatProperty(TargetLodScaleFactorInfo, 15.f, 1.f, 50.f), FloatProperty(CurrentLodScaleFactorInfo, 15.f, 1.f, 50.f), - FloatProperty(CameraMinHeightInfo, 100.f, 0.f, 1000.f), FloatProperty(OrenNayarRoughnessInfo, 0.f, 0.f, 1.f), IntProperty(NActiveLayersInfo, 0, 0, OpenGLCap.maxTextureUnits() / 3) }) @@ -605,16 +592,11 @@ RenderableGlobe::RenderableGlobe(const ghoul::Dictionary& dictionary) }); addProperty(_generalProperties.targetLodScaleFactor); addProperty(_generalProperties.currentLodScaleFactor); - addProperty(_generalProperties.cameraMinHeight); addProperty(_generalProperties.orenNayarRoughness); _generalProperties.nActiveLayers.setReadOnly(true); addProperty(_generalProperties.nActiveLayers); _debugPropertyOwner.addProperty(_debugProperties.showChunkEdges); - //_debugPropertyOwner.addProperty(_debugProperties.showChunkBounds); - //_debugPropertyOwner.addProperty(_debugProperties.showChunkAABB); - //_debugPropertyOwner.addProperty(_debugProperties.showHeightResolution); - //_debugPropertyOwner.addProperty(_debugProperties.showHeightIntensities); _debugPropertyOwner.addProperty(_debugProperties.levelByProjectedAreaElseDistance); _debugPropertyOwner.addProperty(_debugProperties.resetTileProviders); _debugPropertyOwner.addProperty(_debugProperties.modelSpaceRenderingCutoffLevel); @@ -628,8 +610,6 @@ RenderableGlobe::RenderableGlobe(const ghoul::Dictionary& dictionary) _generalProperties.eclipseHardShadows.onChange(notifyShaderRecompilation); _generalProperties.performShading.onChange(notifyShaderRecompilation); _debugProperties.showChunkEdges.onChange(notifyShaderRecompilation); - _debugProperties.showHeightResolution.onChange(notifyShaderRecompilation); - _debugProperties.showHeightIntensities.onChange(notifyShaderRecompilation); _layerManager.onChange([&](Layer* l) { _shadersNeedRecompilation = true; @@ -808,13 +788,6 @@ void RenderableGlobe::update(const UpdateData& data) { _localRenderer.program->setUniform("xSegments", _grid.xSegments); - if (_debugProperties.showHeightResolution) { - _localRenderer.program->setUniform( - "vertexResolution", - glm::vec2(_grid.xSegments, _grid.ySegments) - ); - } - ghoul::opengl::updateUniformLocations( *_localRenderer.program, _localRenderer.uniformCache, @@ -827,12 +800,6 @@ void RenderableGlobe::update(const UpdateData& data) { _globalRenderer.program->setUniform("xSegments", _grid.xSegments); - if (_debugProperties.showHeightResolution) { - _globalRenderer.program->setUniform( - "vertexResolution", - glm::vec2(_grid.xSegments, _grid.ySegments) - ); - } // Ellipsoid Radius (Model Space) _globalRenderer.program->setUniform( "radiiSquared", @@ -1220,24 +1187,6 @@ void RenderableGlobe::renderChunks(const RenderData& data, RendererTasks&, } _localRenderer.program->deactivate(); - if (_debugProperties.showChunkBounds) { - for (int i = 0; i < globalCount; ++i) { - debugRenderChunk( - *_globalChunkBuffer[i], - mvp, - _debugProperties.showChunkBounds - ); - } - - for (int i = 0; i < localCount; ++i) { - debugRenderChunk( - *_localChunkBuffer[i], - mvp, - _debugProperties.showChunkBounds - ); - } - } - // If our tile cache is very full, we assume we need to adjust the level of detail // dynamically to not keep rendering frames with unavailable data // After certain number of iterations(_debugProperties.dynamicLodIterationCount) of @@ -1658,12 +1607,8 @@ void RenderableGlobe::recompileShaders() { std::to_string(_generalProperties.shadowMapping) ); pairs.emplace_back("showChunkEdges", std::to_string(_debugProperties.showChunkEdges)); - pairs.emplace_back("showHeightResolution", - std::to_string(_debugProperties.showHeightResolution) - ); - pairs.emplace_back("showHeightIntensities", - std::to_string(_debugProperties.showHeightIntensities) - ); + pairs.emplace_back("showHeightResolution", "0"); + pairs.emplace_back("showHeightIntensities", "0"); pairs.emplace_back("defaultHeight", std::to_string(DefaultHeight)); @@ -1771,13 +1716,6 @@ void RenderableGlobe::recompileShaders() { _localRenderer.program->setUniform("xSegments", _grid.xSegments); - if (_debugProperties.showHeightResolution) { - _localRenderer.program->setUniform( - "vertexResolution", - glm::vec2(_grid.xSegments, _grid.ySegments) - ); - } - ghoul::opengl::updateUniformLocations( *_localRenderer.program, _localRenderer.uniformCache, @@ -1799,12 +1737,6 @@ void RenderableGlobe::recompileShaders() { _globalRenderer.program->setUniform("xSegments", _grid.xSegments); - if (_debugProperties.showHeightResolution) { - _globalRenderer.program->setUniform( - "vertexResolution", - glm::vec2(_grid.xSegments, _grid.ySegments) - ); - } // Ellipsoid Radius (Model Space) _globalRenderer.program->setUniform( "radiiSquared", diff --git a/modules/globebrowsing/src/renderableglobe.h b/modules/globebrowsing/src/renderableglobe.h index c1ce8d7a98..35247cefe7 100644 --- a/modules/globebrowsing/src/renderableglobe.h +++ b/modules/globebrowsing/src/renderableglobe.h @@ -123,9 +123,6 @@ private: struct { properties::BoolProperty showChunkEdges; - properties::BoolProperty showChunkBounds; - properties::BoolProperty showHeightResolution; - properties::BoolProperty showHeightIntensities; properties::BoolProperty levelByProjectedAreaElseDistance; properties::BoolProperty resetTileProviders; properties::IntProperty modelSpaceRenderingCutoffLevel; @@ -142,7 +139,6 @@ private: properties::IntProperty nShadowSamples; properties::FloatProperty targetLodScaleFactor; properties::FloatProperty currentLodScaleFactor; - properties::FloatProperty cameraMinHeight; properties::FloatProperty orenNayarRoughness; properties::IntProperty nActiveLayers; } _generalProperties; From f74bf975131c21189ece7b8a495f887bf8de242f Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 8 May 2021 16:57:08 +0200 Subject: [PATCH 3/8] Update SGCT repository (closes #1437), VAO hygiene for RenderablePlane --- apps/OpenSpace/ext/sgct | 2 +- modules/base/rendering/renderableplane.cpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/OpenSpace/ext/sgct b/apps/OpenSpace/ext/sgct index 669fbc16a9..2a3ef78f72 160000 --- a/apps/OpenSpace/ext/sgct +++ b/apps/OpenSpace/ext/sgct @@ -1 +1 @@ -Subproject commit 669fbc16a9910b28333427f5e07235baa43ea7a6 +Subproject commit 2a3ef78f721e919531bb05b0cedab2dabe2bb0be diff --git a/modules/base/rendering/renderableplane.cpp b/modules/base/rendering/renderableplane.cpp index a029a4f576..40c9e57281 100644 --- a/modules/base/rendering/renderableplane.cpp +++ b/modules/base/rendering/renderableplane.cpp @@ -275,6 +275,7 @@ void RenderablePlane::render(const RenderData& data, RendererTasks&) { glBindVertexArray(_quad); glDrawArrays(GL_TRIANGLES, 0, 6); + glBindVertexArray(0); if (additiveBlending) { glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); @@ -327,6 +328,7 @@ void RenderablePlane::createPlane() { sizeof(GLfloat) * 6, reinterpret_cast(sizeof(GLfloat) * 4) ); + glBindVertexArray(0); } } // namespace openspace From 09e28ff49e362e1dd991da26a18dc3c8c8e4fc3d Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 8 May 2021 18:08:56 +0200 Subject: [PATCH 4/8] Enable Screenspace renderable to have a multiplicative color; Add new asset to show a target marker (closes #85); Fix bug with wrong documentation shown in RenderablePlane --- data/assets/util/add_marker.asset | 19 +++++++++++++++ .../rendering/screenspacerenderable.h | 3 ++- modules/base/rendering/renderableplane.cpp | 2 +- .../rendering/renderableplaneimageonline.cpp | 2 -- .../base/rendering/screenspaceimageonline.cpp | 2 +- modules/base/shaders/screenspace_fs.glsl | 6 ++--- src/rendering/screenspacerenderable.cpp | 24 +++++++++++++++---- 7 files changed, 45 insertions(+), 13 deletions(-) create mode 100644 data/assets/util/add_marker.asset diff --git a/data/assets/util/add_marker.asset b/data/assets/util/add_marker.asset new file mode 100644 index 0000000000..1d62a2f00c --- /dev/null +++ b/data/assets/util/add_marker.asset @@ -0,0 +1,19 @@ +local icons = asset.syncedResource({ + Name = "Icons", + Type = "HttpSynchronization", + Identifier = "icons", + Version = 1 +}) + +asset.onInitialize(function() + openspace.addScreenSpaceRenderable({ + Identifier = "target-marker", + Name = "Target Marker", + Type = "ScreenSpaceImageLocal", + TexturePath = icons .. '/target.png' + }) +end) + +asset.onDeinitialize(function() + openspace.removeScreenSpaceRenderable('target-marker'); +end) diff --git a/include/openspace/rendering/screenspacerenderable.h b/include/openspace/rendering/screenspacerenderable.h index 189ae16d12..bffec324c4 100644 --- a/include/openspace/rendering/screenspacerenderable.h +++ b/include/openspace/rendering/screenspacerenderable.h @@ -104,11 +104,12 @@ protected: properties::Vec3Property _localRotation; properties::FloatProperty _scale; + properties::Vec3Property _multiplyColor; properties::FloatProperty _opacity; properties::TriggerProperty _delete; glm::ivec2 _objectSize = glm::ivec2(0); - UniformCache(alpha, modelTransform, viewProj, texture) _uniformCache; + UniformCache(color, alpha, modelTransform, viewProj, texture) _uniformCache; std::unique_ptr _shader; }; diff --git a/modules/base/rendering/renderableplane.cpp b/modules/base/rendering/renderableplane.cpp index 40c9e57281..52cbaf638e 100644 --- a/modules/base/rendering/renderableplane.cpp +++ b/modules/base/rendering/renderableplane.cpp @@ -91,7 +91,7 @@ namespace { // [[codegen::verbatim(BlendModeInfo.description)]] std::optional blendMode; - // [[codegen::verbatim(BlendModeInfo.description)]] + // [[codegen::verbatim(MultiplyColorInfo.description)]] std::optional multiplyColor [[codegen::color()]]; }; #include "renderableplane_codegen.cpp" diff --git a/modules/base/rendering/renderableplaneimageonline.cpp b/modules/base/rendering/renderableplaneimageonline.cpp index 368aa34439..a86fbd2f39 100644 --- a/modules/base/rendering/renderableplaneimageonline.cpp +++ b/modules/base/rendering/renderableplaneimageonline.cpp @@ -75,8 +75,6 @@ RenderablePlaneImageOnline::RenderablePlaneImageOnline( const Parameters p = codegen::bake(dictionary); _texturePath.onChange([this]() { _textureIsDirty = true; }); - addProperty(_texturePath); - _texturePath = p.url; addProperty(_texturePath); } diff --git a/modules/base/rendering/screenspaceimageonline.cpp b/modules/base/rendering/screenspaceimageonline.cpp index ef5c527175..9754cc737a 100644 --- a/modules/base/rendering/screenspaceimageonline.cpp +++ b/modules/base/rendering/screenspaceimageonline.cpp @@ -81,8 +81,8 @@ ScreenSpaceImageOnline::ScreenSpaceImageOnline(const ghoul::Dictionary& dictiona setIdentifier(std::move(identifier)); _texturePath.onChange([this]() { _textureIsDirty = true; }); - addProperty(_texturePath); _texturePath = p.url.value_or(_texturePath); + addProperty(_texturePath); } ScreenSpaceImageOnline::~ScreenSpaceImageOnline() {} // NOLINT diff --git a/modules/base/shaders/screenspace_fs.glsl b/modules/base/shaders/screenspace_fs.glsl index 7c0f7e1b57..fabb553c14 100644 --- a/modules/base/shaders/screenspace_fs.glsl +++ b/modules/base/shaders/screenspace_fs.glsl @@ -29,13 +29,13 @@ in vec2 vs_st; in vec4 vs_position; uniform sampler2D texture1; -uniform float Alpha; +uniform vec3 MultiplyColor = vec3(1.0, 1.0, 1.0); +uniform float Alpha = 1.0; Fragment getFragment() { Fragment frag; - frag.color = texture(texture1, vs_st); - frag.color.a = Alpha * frag.color.a; + frag.color = texture(texture1, vs_st) * vec4(MultiplyColor, Alpha); if (frag.color.a == 0.0) { discard; } diff --git a/src/rendering/screenspacerenderable.cpp b/src/rendering/screenspacerenderable.cpp index fa64370e68..c8864fe1a6 100644 --- a/src/rendering/screenspacerenderable.cpp +++ b/src/rendering/screenspacerenderable.cpp @@ -42,8 +42,8 @@ #include namespace { - constexpr const std::array UniformNames = { - "Alpha", "ModelTransform", "ViewProjectionMatrix", "texture1" + constexpr const std::array UniformNames = { + "MultiplyColor", "Alpha", "ModelTransform", "ViewProjectionMatrix", "texture1" }; constexpr openspace::properties::Property::PropertyInfo EnabledInfo = { @@ -100,6 +100,12 @@ namespace { "An euler rotation (x, y, z) to apply to the plane." }; + constexpr openspace::properties::Property::PropertyInfo MultiplyColorInfo = { + "MultiplyColor", + "Multiply Color", + "If set, the plane's texture is multiplied with this color. " + "Useful for applying a color grayscale images." + }; constexpr openspace::properties::Property::PropertyInfo OpacityInfo = { "Opacity", @@ -245,6 +251,9 @@ namespace { // [[codegen::verbatim(UsePerspectiveProjectionInfo.description)]] std::optional usePerspectiveProjection; + // [[codegen::verbatim(MultiplyColorInfo.description)]] + std::optional multiplyColor [[codegen::color()]]; + // [codegen::verbatim(OpacityInfo.description)]] std::optional opacity [[codegen::inrange(0.f, 1.f)]]; @@ -324,6 +333,7 @@ ScreenSpaceRenderable::ScreenSpaceRenderable(const ghoul::Dictionary& dictionary glm::vec3(glm::pi()) ) , _scale(ScaleInfo, 0.25f, 0.f, 2.f) + , _multiplyColor(MultiplyColorInfo, glm::vec3(1.f), glm::vec3(0.f), glm::vec3(1.f)) , _opacity(OpacityInfo, 1.f, 0.f, 1.f) , _delete(DeleteInfo) { @@ -355,9 +365,14 @@ ScreenSpaceRenderable::ScreenSpaceRenderable(const ghoul::Dictionary& dictionary }); addProperty(_scale); + addProperty(_multiplyColor); addProperty(_opacity); addProperty(_localRotation); + + _multiplyColor = p.multiplyColor.value_or(_multiplyColor); + _multiplyColor.setViewOption(properties::Property::ViewOptions::Color); + _enabled = p.enabled.value_or(_enabled); _useRadiusAzimuthElevation = p.useRadiusAzimuthElevation.value_or(_useRadiusAzimuthElevation); @@ -554,6 +569,7 @@ void ScreenSpaceRenderable::draw(glm::mat4 modelTransform) { _shader->activate(); + _shader->setUniform(_uniformCache.color, _multiplyColor); _shader->setUniform(_uniformCache.alpha, _opacity); _shader->setUniform(_uniformCache.modelTransform, modelTransform); @@ -576,8 +592,6 @@ void ScreenSpaceRenderable::draw(glm::mat4 modelTransform) { unbindTexture(); } -void ScreenSpaceRenderable::unbindTexture() { -} - +void ScreenSpaceRenderable::unbindTexture() {} } // namespace openspace From e3af4a9f0912135004f84fc47398ae454d572d57 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 8 May 2021 19:17:00 +0200 Subject: [PATCH 5/8] Add the ability to specifiy a specific time for a TemporalTileProvider (closes #1171) --- modules/globebrowsing/src/tileprovider.cpp | 48 +++++++++++++++++++--- modules/globebrowsing/src/tileprovider.h | 3 ++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/modules/globebrowsing/src/tileprovider.cpp b/modules/globebrowsing/src/tileprovider.cpp index ddf1200ed9..10fa05b65f 100644 --- a/modules/globebrowsing/src/tileprovider.cpp +++ b/modules/globebrowsing/src/tileprovider.cpp @@ -148,6 +148,21 @@ namespace temporal { "This is the path to the XML configuration file that describes the temporal tile " "information." }; + + constexpr openspace::properties::Property::PropertyInfo UseFixedTimeInfo = { + "UseFixedTime", + "Use Fixed Time", + "If this value is enabled, the time-varying timevarying dataset will always use " + "the time that is specified in the 'FixedTime' property, rather than using the " + "actual time from OpenSpace" + }; + + constexpr openspace::properties::Property::PropertyInfo FixedTimeInfo = { + "FixedTime", + "Fixed Time", + "If the 'UseFixedTime' is enabled, this time will be used instead of the actual " + "time taken from OpenSpace for the displayed tiles." + }; } // namespace temporal @@ -375,18 +390,29 @@ TileProvider* getTileProvider(TemporalTileProvider& t, std::string_view timekey) TileProvider* getTileProvider(TemporalTileProvider& t, const Time& time) { ZoneScoped - Time tCopy(time); - if (t.timeQuantizer.quantize(tCopy, true)) { - char Buffer[22]; - const int size = timeStringify(t.timeFormat, tCopy, Buffer); + if (t.useFixedTime && !t.fixedTime.value().empty()) { try { - return getTileProvider(t, std::string_view(Buffer, size)); + return getTileProvider(t, t.fixedTime.value()); } catch (const ghoul::RuntimeError& e) { LERRORC("TemporalTileProvider", e.message); return nullptr; } } + else { + Time tCopy(time); + if (t.timeQuantizer.quantize(tCopy, true)) { + char Buffer[22]; + const int size = timeStringify(t.timeFormat, tCopy, Buffer); + try { + return getTileProvider(t, std::string_view(Buffer, size)); + } + catch (const ghoul::RuntimeError& e) { + LERRORC("TemporalTileProvider", e.message); + return nullptr; + } + } + } return nullptr; } @@ -831,6 +857,8 @@ TileProviderByLevel::TileProviderByLevel(const ghoul::Dictionary& dictionary) { TemporalTileProvider::TemporalTileProvider(const ghoul::Dictionary& dictionary) : initDict(dictionary) , filePath(temporal::FilePathInfo) + , useFixedTime(temporal::UseFixedTimeInfo, false) + , fixedTime(temporal::FixedTimeInfo) { ZoneScoped @@ -839,6 +867,16 @@ TemporalTileProvider::TemporalTileProvider(const ghoul::Dictionary& dictionary) filePath = dictionary.value(KeyFilePath); addProperty(filePath); + if (dictionary.hasValue(temporal::UseFixedTimeInfo.identifier)) { + useFixedTime = dictionary.value(temporal::UseFixedTimeInfo.identifier); + } + addProperty(useFixedTime); + + if (dictionary.hasValue(temporal::FixedTimeInfo.identifier)) { + fixedTime = dictionary.value(temporal::FixedTimeInfo.identifier); + } + addProperty(fixedTime); + successfulInitialization = readFilePath(*this); if (!successfulInitialization) { diff --git a/modules/globebrowsing/src/tileprovider.h b/modules/globebrowsing/src/tileprovider.h index 2d70e8dd0c..0641739388 100644 --- a/modules/globebrowsing/src/tileprovider.h +++ b/modules/globebrowsing/src/tileprovider.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -174,6 +175,8 @@ struct TemporalTileProvider : public TileProvider { ghoul::Dictionary initDict; properties::StringProperty filePath; + properties::BoolProperty useFixedTime; + properties::StringProperty fixedTime; std::string gdalXmlTemplate; std::unordered_map> tileProviderMap; From abd084c0a9e2ed7841fc2966b744b5f8d02298aa Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 8 May 2021 19:53:40 +0200 Subject: [PATCH 6/8] Only use the adaptive level-of-detail when actually rendering out frames in a session recording (closes #1292); Remove OpenGL performance warning messages when writing frames due to expected GPU-CPU sync --- modules/globebrowsing/src/renderableglobe.cpp | 54 ++++++++++--------- openspace.cfg | 2 + 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/modules/globebrowsing/src/renderableglobe.cpp b/modules/globebrowsing/src/renderableglobe.cpp index 7f5b65e2da..eaf7a178e4 100644 --- a/modules/globebrowsing/src/renderableglobe.cpp +++ b/modules/globebrowsing/src/renderableglobe.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -140,7 +141,8 @@ namespace { "DynamicLodIterationCount", "Data availability checks before LOD factor impact", "The number of checks that have to fail/succeed in a row before the dynamic " - "level-of-detail adjusts the actual level-of-detail up or down" + "level-of-detail adjusts the actual level-of-detail up or down during a session " + "recording" }; constexpr openspace::properties::Property::PropertyInfo PerformShadingInfo = { @@ -1187,30 +1189,32 @@ void RenderableGlobe::renderChunks(const RenderData& data, RendererTasks&, } _localRenderer.program->deactivate(); - // If our tile cache is very full, we assume we need to adjust the level of detail - // dynamically to not keep rendering frames with unavailable data - // After certain number of iterations(_debugProperties.dynamicLodIterationCount) of - // unavailable/available data in a row, we assume that a change could be made. - const int iterCount = _debugProperties.dynamicLodIterationCount; - const bool exceededIterations = - static_cast(_iterationsOfUnavailableData) > iterCount; - const float clf = _generalProperties.currentLodScaleFactor; - const float clfMin = _generalProperties.currentLodScaleFactor.minValue(); - const float targetLod = _generalProperties.targetLodScaleFactor; - const bool validLodFactor = clf > clfMin; - if (exceededIterations && validLodFactor) { - _generalProperties.currentLodScaleFactor = - _generalProperties.currentLodScaleFactor - 0.1f; - _iterationsOfUnavailableData = 0; - _lodScaleFactorDirty = true; - } // Make 2 times the iterations with available data to move it up again - else if (static_cast(_iterationsOfAvailableData) > - (iterCount * 2) && clf < targetLod) - { - _generalProperties.currentLodScaleFactor = - _generalProperties.currentLodScaleFactor + 0.1f; - _iterationsOfAvailableData = 0; - _lodScaleFactorDirty = true; + if (global::sessionRecording->isSavingFramesDuringPlayback()) { + // If our tile cache is very full, we assume we need to adjust the level of detail + // dynamically to not keep rendering frames with unavailable data + // After certain number of iterations(_debugProperties.dynamicLodIterationCount) of + // unavailable/available data in a row, we assume that a change could be made. + const int iterCount = _debugProperties.dynamicLodIterationCount; + const bool exceededIterations = + static_cast(_iterationsOfUnavailableData) > iterCount; + const float clf = _generalProperties.currentLodScaleFactor; + const float clfMin = _generalProperties.currentLodScaleFactor.minValue(); + const float targetLod = _generalProperties.targetLodScaleFactor; + const bool validLodFactor = clf > clfMin; + if (exceededIterations && validLodFactor) { + _generalProperties.currentLodScaleFactor = + _generalProperties.currentLodScaleFactor - 0.1f; + _iterationsOfUnavailableData = 0; + _lodScaleFactorDirty = true; + } // Make 2 times the iterations with available data to move it up again + else if (static_cast(_iterationsOfAvailableData) > + (iterCount * 2) && clf < targetLod) + { + _generalProperties.currentLodScaleFactor = + _generalProperties.currentLodScaleFactor + 0.1f; + _iterationsOfAvailableData = 0; + _lodScaleFactorDirty = true; + } } } diff --git a/openspace.cfg b/openspace.cfg index 9805780f2d..140c040ff3 100644 --- a/openspace.cfg +++ b/openspace.cfg @@ -234,6 +234,8 @@ OpenGLDebugContext = { { Type = "Other", Source = "API", Identifier = 131185 }, -- API_ID_RECOMPILE_FRAGMENT_SHADER performance warning has been generated. Fragment shader recompiled due to state change { Type = "Performance", Source = "API", Identifier = 2 }, + -- Pixel-path performance warning: Pixel transfer is synchronized with 3D rendering + { Type = "Performance", Source = "API", Identifier = 131154 }, -- Buffer performance warning: "copied/moved from VIDEO memory to HOST memory" { Type = "Performance", Source = "API", Identifier = 131186 }, -- API_ID_LINE_WIDTH deprecated behavior warning has been generated From 9b1f733deb6f3207eac8415181a4c179b60868f5 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 8 May 2021 20:18:18 +0200 Subject: [PATCH 7/8] Fix issue that prevented default-constructed transformations from being registered as PropertyOwners --- src/scene/scenegraphnode.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/scene/scenegraphnode.cpp b/src/scene/scenegraphnode.cpp index caac1536e3..b1bfdbcf2e 100644 --- a/src/scene/scenegraphnode.cpp +++ b/src/scene/scenegraphnode.cpp @@ -306,7 +306,6 @@ ghoul::mm_unique_ptr SceneGraphNode::createFromDictionary( LDEBUG(fmt::format( "Successfully created ephemeris for '{}'", result->identifier() )); - result->addPropertySubOwner(result->_transform.translation.get()); } if (p.transform->rotation.has_value()) { @@ -326,7 +325,6 @@ ghoul::mm_unique_ptr SceneGraphNode::createFromDictionary( LDEBUG(fmt::format( "Successfully created rotation for '{}'", result->identifier() )); - result->addPropertySubOwner(result->_transform.rotation.get()); } if (p.transform->scale.has_value()) { @@ -344,9 +342,12 @@ ghoul::mm_unique_ptr SceneGraphNode::createFromDictionary( LDEBUG(fmt::format( "Successfully created scale for '{}'", result->identifier() )); - result->addPropertySubOwner(result->_transform.scale.get()); } } + result->addPropertySubOwner(result->_transform.translation.get()); + result->addPropertySubOwner(result->_transform.rotation.get()); + result->addPropertySubOwner(result->_transform.scale.get()); + if (p.timeFrame.has_value()) { result->_timeFrame = TimeFrame::createFromDictionary(*p.timeFrame); From 2ca7101b6c4843bae48d453f1b1aaa3b63429742 Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sat, 8 May 2021 21:25:35 +0200 Subject: [PATCH 8/8] Actually make use of the model matrix in the atmosphere radius calculation for determining whether it should be culled (closes #1465) --- modules/atmosphere/rendering/atmospheredeferredcaster.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/atmosphere/rendering/atmospheredeferredcaster.cpp b/modules/atmosphere/rendering/atmospheredeferredcaster.cpp index 0a27be4596..049279ca5b 100644 --- a/modules/atmosphere/rendering/atmospheredeferredcaster.cpp +++ b/modules/atmosphere/rendering/atmospheredeferredcaster.cpp @@ -113,7 +113,7 @@ namespace { constexpr const char* GlslDeferredcastVsPath = "${MODULES}/atmosphere/shaders/atmosphere_deferred_vs.glsl"; - constexpr const float ATM_EPS = 2.f; + constexpr const float ATM_EPS = 2000.f; constexpr const float KM_TO_M = 1000.f; @@ -213,7 +213,7 @@ void AtmosphereDeferredcaster::preRaycast(const RenderData& renderData, renderData.camera.sgctInternal.projectionMatrix() ) * renderData.camera.combinedViewMatrix(); - const float totalAtmosphere = (_atmosphereRadius + ATM_EPS)* KM_TO_M; + const float totalAtmosphere = (scaledRadius + ATM_EPS); if (!isAtmosphereInFrustum(MV, tPlanetPosWorld, totalAtmosphere)) { program.setUniform(_uniformCache.cullAtmosphere, 1); }