From 83c7d34df27ec191b06959ac52bc8cddea9d95c2 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 11 Aug 2017 18:40:16 -0400 Subject: [PATCH 1/9] Switched to populating the sector cache with everything in a track the first time anything on that track is requested. That avoids the problem whereby each request of a non-existent sector costs two spins. --- StaticAnalyser/AmstradCPC/StaticAnalyser.cpp | 2 ++ Storage/Disk/Encodings/MFM.cpp | 30 ++++++++++++++------ Storage/Disk/Encodings/MFM.hpp | 1 + 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp b/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp index 8fcb1839e..bdf536ed6 100644 --- a/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp +++ b/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp @@ -66,6 +66,8 @@ static void InspectDataCatalogue( static void InspectSystemCatalogue( const std::unique_ptr &data_catalogue, StaticAnalyser::Target &target) { + // TODO: does track 0, side 0, sector 0x41 exist? If not, InspectDataCatalogue. + // If this is a system disk, then launch it as though it were CP/M. target.loadingCommand = "|cpm\n"; } diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index aab85facb..a49f3f40b 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -266,6 +266,26 @@ void Parser::seek_to_track(uint8_t track) { } std::shared_ptr Parser::get_sector(uint8_t head, uint8_t track, uint8_t sector) { + // Switch head and track if necessary. + if(head_ != head) { + drive_->set_head(head); + invalidate_track(); + } + seek_to_track(track); + int track_index = get_index(head, track, 0); + + // Populate the sector cache if it's not already populated. + if(decoded_tracks_.find(track_index) == decoded_tracks_.end()) { + std::shared_ptr first_sector = get_next_sector(); + if(first_sector) { + while(1) { + std::shared_ptr next_sector = get_next_sector(); + if(next_sector->sector == first_sector->sector) break; + } + } + decoded_tracks_.insert(track_index); + } + // Check cache for sector. int index = get_index(head, track, sector); auto cached_sector = sectors_by_index_.find(index); @@ -273,14 +293,8 @@ std::shared_ptr Parser::get_sector(uint8_t head, uint8_t track, uint8_t return cached_sector->second; } - // Failing that, set the proper head and track, and search for the sector. get_sector automatically - // inserts everything found into sectors_by_index_. - if(head_ != head) { - drive_->set_head(head); - invalidate_track(); - } - seek_to_track(track); - return get_sector(sector); + // If it wasn't found, it doesn't exist. + return nullptr; } std::vector Parser::get_track(uint8_t track) { diff --git a/Storage/Disk/Encodings/MFM.hpp b/Storage/Disk/Encodings/MFM.hpp index eed23e5b3..2ae651555 100644 --- a/Storage/Disk/Encodings/MFM.hpp +++ b/Storage/Disk/Encodings/MFM.hpp @@ -140,6 +140,7 @@ class Parser: public Storage::Disk::Controller { std::vector get_track(); std::map> sectors_by_index_; + std::set decoded_tracks_; int get_index(uint8_t head, uint8_t track, uint8_t sector); }; From 0c8769e33508ce7b13db3ecfed3b2b3afa8579cc Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 11 Aug 2017 18:41:08 -0400 Subject: [PATCH 2/9] Just to be safe. --- Storage/Disk/Encodings/MFM.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Storage/Disk/Encodings/MFM.cpp b/Storage/Disk/Encodings/MFM.cpp index a49f3f40b..aeb47404a 100644 --- a/Storage/Disk/Encodings/MFM.cpp +++ b/Storage/Disk/Encodings/MFM.cpp @@ -280,7 +280,7 @@ std::shared_ptr Parser::get_sector(uint8_t head, uint8_t track, uint8_t if(first_sector) { while(1) { std::shared_ptr next_sector = get_next_sector(); - if(next_sector->sector == first_sector->sector) break; + if(!next_sector || next_sector->sector == first_sector->sector) break; } } decoded_tracks_.insert(track_index); From 7e35e449346d132ba4886e51ec0bc892c43a6610 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 11 Aug 2017 18:46:39 -0400 Subject: [PATCH 3/9] Added an extra sanity check on treating system disks as system disks. --- StaticAnalyser/AmstradCPC/StaticAnalyser.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp b/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp index bdf536ed6..676bc729e 100644 --- a/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp +++ b/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp @@ -7,7 +7,9 @@ // #include "StaticAnalyser.hpp" + #include "../../Storage/Disk/Parsers/CPM.hpp" +#include "../../Storage/Disk/Encodings/MFM.hpp" static bool strcmp_insensitive(const char *a, const char *b) { if(strlen(a) != strlen(b)) return false; @@ -64,12 +66,17 @@ static void InspectDataCatalogue( } static void InspectSystemCatalogue( - const std::unique_ptr &data_catalogue, + const std::shared_ptr &disk, + const std::unique_ptr &catalogue, StaticAnalyser::Target &target) { - // TODO: does track 0, side 0, sector 0x41 exist? If not, InspectDataCatalogue. - - // If this is a system disk, then launch it as though it were CP/M. - target.loadingCommand = "|cpm\n"; + Storage::Encodings::MFM::Parser parser(true, disk); + // Check that the boot sector exists. + if(parser.get_sector(0, 0, 0x41) != nullptr) { + // This is a system disk, then launch it as though it were CP/M. + target.loadingCommand = "|cpm\n"; + } else { + InspectDataCatalogue(catalogue, target); + } } void StaticAnalyser::AmstradCPC::AddTargets( @@ -116,7 +123,7 @@ void StaticAnalyser::AmstradCPC::AddTargets( std::unique_ptr system_catalogue = Storage::Disk::CPM::GetCatalogue(target.disks.front(), system_format); if(system_catalogue) { - InspectSystemCatalogue(data_catalogue, target); + InspectSystemCatalogue(target.disks.front(), data_catalogue, target); } } } From ffb1a14ace8a7070a16f1a87633223d7dbcb5844 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 11 Aug 2017 18:56:33 -0400 Subject: [PATCH 4/9] Minor: clear status registers before a read data. --- Components/8272/i8272.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Components/8272/i8272.cpp b/Components/8272/i8272.cpp index e4c68be49..614e5923f 100644 --- a/Components/8272/i8272.cpp +++ b/Components/8272/i8272.cpp @@ -247,6 +247,7 @@ void i8272::posit_event(int event_type) { // Establishes the drive and head being addressed, and whether in double density mode; populates the internal // cylinder, head, sector and size registers from the command stream. + status_[0] = status_[1] = status_[2] = 0; SET_DRIVE_HEAD_MFM(); cylinder_ = command_[2]; head_ = command_[3]; From bfe297052ddc032fe6ccab66e3f42916265eaea5 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 11 Aug 2017 18:59:38 -0400 Subject: [PATCH 5/9] Picked up another subtlety: disk names may be outside of the ones a user could type, in which case they definitely don't affect the decision. --- StaticAnalyser/AmstradCPC/StaticAnalyser.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp b/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp index 676bc729e..da34bfabe 100644 --- a/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp +++ b/StaticAnalyser/AmstradCPC/StaticAnalyser.cpp @@ -39,6 +39,10 @@ static void InspectDataCatalogue( size_t last_implicit_suffixed_file = 0; for(size_t c = 0; c < data_catalogue->files.size(); c++) { + // Files with nothing but spaces in their name can't be loaded by the user, so disregard them. + if(data_catalogue->files[c].type == " " && data_catalogue->files[c].name == " ") + continue; + // Check for whether this is [potentially] BASIC. if(data_catalogue->files[c].data.size() >= 128 && !((data_catalogue->files[c].data[18] >> 1) & 7)) { basic_files++; From 82ca49c840091663d20a626230b51d42b46f40b8 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 11 Aug 2017 19:05:46 -0400 Subject: [PATCH 6/9] Adjusted to avoid calls to `::greatest_common_divisor(numerator % denominator, denominator)` unless necessary. --- Storage/TimedEventLoop.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Storage/TimedEventLoop.cpp b/Storage/TimedEventLoop.cpp index 7badd9ee5..377af80fd 100644 --- a/Storage/TimedEventLoop.cpp +++ b/Storage/TimedEventLoop.cpp @@ -43,10 +43,12 @@ void TimedEventLoop::set_next_event_time_interval(Time interval) { (int64_t)subcycles_until_event_.clock_rate * (int64_t)input_clock_rate_ * (int64_t)interval.length + (int64_t)interval.clock_rate * (int64_t)subcycles_until_event_.length; - // Simplify now, to prepare for stuffing into possibly 32-bit quantities - int64_t common_divisor = NumberTheory::greatest_common_divisor(numerator % denominator, denominator); - denominator /= common_divisor; - numerator /= common_divisor; + // Simplify if necessary. + if(denominator > std::numeric_limits::max()) { + int64_t common_divisor = NumberTheory::greatest_common_divisor(numerator % denominator, denominator); + denominator /= common_divisor; + numerator /= common_divisor; + } // So this event will fire in the integral number of cycles from now, putting us at the remainder // number of subcycles From 2d81acb82e1001ef9135f122ea87a8ef3e6a486b Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 11 Aug 2017 19:18:45 -0400 Subject: [PATCH 7/9] Upped C++ standard to C++14 and added an #if that's intended to use the built-in std::gcd when compiled on C++17 or better. Fixed for new signedness warnings resulting for taking the step to C++14. --- NumberTheory/Factors.hpp | 5 +++++ OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj | 2 ++ Outputs/CRT/Internals/Shaders/Shader.cpp | 4 ++-- Storage/FileHolder.cpp | 2 +- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/NumberTheory/Factors.hpp b/NumberTheory/Factors.hpp index 9fe660bc2..ad21a362f 100644 --- a/NumberTheory/Factors.hpp +++ b/NumberTheory/Factors.hpp @@ -9,6 +9,7 @@ #ifndef Factors_hpp #define Factors_hpp +#include #include namespace NumberTheory { @@ -16,6 +17,9 @@ namespace NumberTheory { @returns The greatest common divisor of @c a and @c b as computed by Euclid's algorithm. */ template T greatest_common_divisor(T a, T b) { +#if __cplusplus > 201402L + return std::gcd(a, b); +#else // TODO: replace with the C++17 GCD function, once available. if(a < b) { std::swap(a, b); @@ -29,6 +33,7 @@ namespace NumberTheory { a = b; b = remainder; } +#endif } /*! diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj index f7a8f670a..b77bb0dd8 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj +++ b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj @@ -3037,6 +3037,7 @@ isa = XCBuildConfiguration; buildSettings = { ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; + CLANG_CXX_LANGUAGE_STANDARD = "c++14"; CLANG_ENABLE_MODULES = YES; CLANG_WARN_ASSIGN_ENUM = YES; CLANG_WARN_DOCUMENTATION_COMMENTS = YES; @@ -3065,6 +3066,7 @@ isa = XCBuildConfiguration; buildSettings = { ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; + CLANG_CXX_LANGUAGE_STANDARD = "c++14"; CLANG_ENABLE_MODULES = YES; CLANG_WARN_ASSIGN_ENUM = YES; CLANG_WARN_DOCUMENTATION_COMMENTS = YES; diff --git a/Outputs/CRT/Internals/Shaders/Shader.cpp b/Outputs/CRT/Internals/Shaders/Shader.cpp index 7d0c0a6b5..413932dc3 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.cpp +++ b/Outputs/CRT/Internals/Shaders/Shader.cpp @@ -29,7 +29,7 @@ GLuint Shader::compile_shader(const std::string &source, GLenum type) { GLint logLength; glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &logLength); if(logLength > 0) { - GLchar *log = new GLchar[logLength]; + GLchar *log = new GLchar[(size_t)logLength]; glGetShaderInfoLog(shader, logLength, &logLength, log); printf("Compile log:\n%s\n", log); delete[] log; @@ -66,7 +66,7 @@ Shader::Shader(const std::string &vertex_shader, const std::string &fragment_sha GLint logLength; glGetProgramiv(shader_program_, GL_INFO_LOG_LENGTH, &logLength); if(logLength > 0) { - GLchar *log = new GLchar[logLength]; + GLchar *log = new GLchar[(size_t)logLength]; glGetProgramInfoLog(shader_program_, logLength, &logLength, log); printf("Link log:\n%s\n", log); delete[] log; diff --git a/Storage/FileHolder.cpp b/Storage/FileHolder.cpp index f447fadd8..9f78253ab 100644 --- a/Storage/FileHolder.cpp +++ b/Storage/FileHolder.cpp @@ -82,7 +82,7 @@ void FileHolder::ensure_file_is_at_least_length(long length) { long bytes_to_write = length - ftell(file_); if(bytes_to_write > 0) { - uint8_t *empty = new uint8_t[bytes_to_write]; + uint8_t *empty = new uint8_t[(size_t)bytes_to_write]; memset(empty, 0, (size_t)bytes_to_write); fwrite(empty, sizeof(uint8_t), (size_t)bytes_to_write, file_); delete[] empty; From daafebe7ace53afe042ee9c07c803e73d90c7290 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 11 Aug 2017 19:19:04 -0400 Subject: [PATCH 8/9] Moved curly bracket. --- Storage/FileHolder.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Storage/FileHolder.cpp b/Storage/FileHolder.cpp index 9f78253ab..6aa66094d 100644 --- a/Storage/FileHolder.cpp +++ b/Storage/FileHolder.cpp @@ -80,8 +80,7 @@ uint16_t FileHolder::fgetc16be() { void FileHolder::ensure_file_is_at_least_length(long length) { fseek(file_, 0, SEEK_END); long bytes_to_write = length - ftell(file_); - if(bytes_to_write > 0) - { + if(bytes_to_write > 0) { uint8_t *empty = new uint8_t[(size_t)bytes_to_write]; memset(empty, 0, (size_t)bytes_to_write); fwrite(empty, sizeof(uint8_t), (size_t)bytes_to_write, file_); From 69914faf02e91f9ef7cdfd42ba8a9c79910a96b5 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 11 Aug 2017 20:22:14 -0400 Subject: [PATCH 9/9] Fixed comments. --- NumberTheory/Factors.hpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/NumberTheory/Factors.hpp b/NumberTheory/Factors.hpp index ad21a362f..ad73315d5 100644 --- a/NumberTheory/Factors.hpp +++ b/NumberTheory/Factors.hpp @@ -14,13 +14,12 @@ namespace NumberTheory { /*! - @returns The greatest common divisor of @c a and @c b as computed by Euclid's algorithm. + @returns The greatest common divisor of @c a and @c b. */ template T greatest_common_divisor(T a, T b) { #if __cplusplus > 201402L return std::gcd(a, b); #else - // TODO: replace with the C++17 GCD function, once available. if(a < b) { std::swap(a, b); } @@ -37,8 +36,8 @@ namespace NumberTheory { } /*! - @returns The least common multiple of @c a and @c b computed indirectly via Euclid's greatest - common divisor algorithm. + @returns The least common multiple of @c a and @c b computed indirectly via the greatest + common divisor. */ template T least_common_multiple(T a, T b) { if(a == b) return a;